Page MenuHomePhabricator

[globalisel][tablegen] Implement unindexed load, non-extending load, and MemVT checks
ClosedPublic

Authored by dsanders on Sep 5 2017, 1:42 AM.

Details

Summary

This includes some context-sensitivity in the MVT to LLT conversion so that
pointer types are tested correctly.
FIXME: I'm not happy with the way this is done since everything is a

special-case. I've yet to find a reasonable way to implement it.

select-load.mir fails because <1 x s64> loads in tablegen get priority over s64
loads. This is fixed in the next patch and as such they should be committed
together, I've posted them separately to help with the review.

Depends on D37456

Diff Detail

Repository
rL LLVM

Event Timeline

dsanders created this revision.Sep 5 2017, 1:42 AM
dsanders updated this revision to Diff 118460.Oct 10 2017, 1:21 PM

Rebase and ping

qcolombet added inline comments.Oct 12 2017, 12:45 PM
include/llvm/CodeGen/GlobalISel/InstructionSelector.h
124 ↗(On Diff #118460)

Doesn't the pointer size depend on the address space?
Thus, given we specific the size this is not really any address space, isn't it?

Basically what I am saying is either don't put the size or change the name of the operator.
BTW, shouldn't we already check the size when checking the type?

include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
247 ↗(On Diff #118460)

Could you add a tablegen test case for this operator?

dsanders added inline comments.Oct 12 2017, 2:21 PM
include/llvm/CodeGen/GlobalISel/InstructionSelector.h
124 ↗(On Diff #118460)

Hmm, I agree that it makes sense for pointer sizes to vary between address spaces. However, iPTR doesn't do anything about address spaces so I don't think SelectionDAG rules account for it. I'll have another look and see what I can find out.

BTW, shouldn't we already check the size when checking the type?

This opcode is used instead of GIM_CheckType for pointers. Pointers are a bit tricky since we have to call a target hook to find out the size of a pointer so we don't know the type until run-time.

include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
247 ↗(On Diff #118460)

Ok

dsanders updated this revision to Diff 119101.Oct 15 2017, 5:56 PM

Refresh before commit

This revision was automatically updated to reflect the committed changes.