[globalisel][tablegen] Add support for fpimm and import of APInt/APFloat based ImmLeaf.
ClosedPublic

Authored by dsanders on Aug 10 2017, 1:37 AM.

Details

Summary

There's only a tablegen testcase for IntImmLeaf and not a CodeGen one
because the relevant rules are rejected for other reasons at the moment.
On AArch64, it's because there's an SDNodeXForm attached to the operand.
On X86, it's because the rule either emits multiple instructions or has
another predicate using PatFrag which cannot easily be supported at the
same time.

dsanders created this revision.Aug 10 2017, 1:37 AM
dsanders updated this revision to Diff 118419.Tue, Oct 10, 10:40 AM

Rebase and ping

qcolombet accepted this revision.Thu, Oct 12, 11:27 AM

LGTM with nitpicks.

Also, when you commit, I would do the AArch64 and X86 changes in different commits.

lib/Target/AArch64/AArch64InstructionSelector.cpp
709

I didn't get that comment.
Could you elaborate?

utils/TableGen/CodeGenDAGPatterns.h
485

Could we move these in GlobalISelEmitter?
Something like GlobalISelEmitter::getPredicateOpcode(TreePredicateFn)?

495

Ditto.

This revision is now accepted and ready to land.Thu, Oct 12, 11:27 AM

Thanks for the reviews

lib/Target/AArch64/AArch64InstructionSelector.cpp
709

This patch causes the rules involving fpimm0 to be imported so we no longer need to handle that case in the C++. It's not harmful to leave it in, but doing so means we can't tell if the test case passes because tablegen works or because the C++ caught it. The comment is intended to explain why we choose not to handle 0.0 in the C++ code even though it would be valid.

utils/TableGen/CodeGenDAGPatterns.h
485

Ok

495

Ok

qcolombet added inline comments.Thu, Oct 12, 2:16 PM
lib/Target/AArch64/AArch64InstructionSelector.cpp
709

Make sense.
Say all that in the comment then :).

dsanders marked 5 inline comments as done.Fri, Oct 13, 2:11 PM
dsanders added inline comments.
utils/TableGen/CodeGenDAGPatterns.h
495

Looking at this one a bit closer, I'm not sure I should move it. It makes use of other private members (immCodeUsesAPInt and immCodeUsesAPFloat) so I'd have to make those public to be able to move it. I don't think those other members should be public since they describe properties of getImmCode() which is also private.

Would it be ok with you to leave this one here?

qcolombet added inline comments.Fri, Oct 13, 2:22 PM
utils/TableGen/CodeGenDAGPatterns.h
495

Yep that's fine.
This one doesn't say GISel ;).

dsanders added inline comments.Fri, Oct 13, 2:26 PM
utils/TableGen/CodeGenDAGPatterns.h
495

Thanks

dsanders closed this revision.Fri, Oct 13, 2:28 PM