This is an archive of the discontinued LLVM Phabricator instance.

X86: Introduce the "relocImm" ComplexPattern, which represents a relocatable immediate.
ClosedPublic

Authored by pcc on Oct 19 2016, 7:50 PM.

Details

Summary

A relocatable immediate is either an immediate operand or an operand that
can be relocated by the linker to an immediate, such as a regular symbol
in non-PIC code.

Start using relocImm for 32-bit and 64-bit MOV instructions, and for operands
of type "imm32_su". Remove a number of now-redundant patterns.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc updated this revision to Diff 75264.Oct 19 2016, 7:50 PM
pcc retitled this revision from to X86: Introduce the "relocImm" ComplexPattern, which represents a relocatable immediate..
pcc updated this object.
pcc added reviewers: lattner, chandlerc.
pcc added a subscriber: llvm-commits.
lattner edited edge metadata.Oct 25 2016, 10:08 PM

This seems like a huge improvement over the status quo. Unfortunately, it's been a couple of years since I've worked on this area though, so I'd like you to get someone working on the X86 backend to look at it.

For example, the change to CodeGenDAGPatterns.cpp is probably necessary given the current tblgen support, but it is better to solve the fix and introduce a pattern cost field to ComplexPattern. I'd have it default to -1 (in which case tblgen would use the default "numoperands*3" heuristic), but allow this to be overridden by def's like relocImm that want to specify a specific complexity to use.

pcc updated this revision to Diff 76152.Oct 27 2016, 6:07 PM
pcc edited edge metadata.
  • Make the complexity a ComplexPattern parameter instead of hardcoding one for relocImm

Done. I'm adding some X86 backend people to the review.

RKSimon added inline comments.Oct 28 2016, 7:20 AM
llvm/utils/TableGen/CodeGenTarget.h
201 ↗(On Diff #76152)

Initialize Complexity to 0? To guarantee that it still matches 'NumOperands * 3'

Thanks for improving this to allow programatic complexity! This looks overall great to me, but again, I'm not the best technical reviewer for X86 backend anymore.

pcc updated this revision to Diff 76274.Oct 28 2016, 4:52 PM
  • Remove the default ComplexPattern constructor
llvm/utils/TableGen/CodeGenTarget.h
201 ↗(On Diff #76152)

It looks like we don't need this constructor at all, so I've removed it.

RKSimon edited edge metadata.Nov 8 2016, 12:48 PM

I'm a bit worried about not having a default constructor at all - since ComplexPattern is used in std::map isn't it likely that these will occur?

pcc added a comment.Nov 8 2016, 12:58 PM

http://llvm-cs.pcc.me.uk/utils/TableGen/CodeGenDAGPatterns.h/rComplexPatterns

We only call insert(), find() and count() on this map, none of which require a default ctor.

Of course, some stdlib implementation may still require the default ctor for some reason. If it does, the bots will tell us and the fix will be easy.

craig.topper accepted this revision.Nov 8 2016, 9:54 PM
craig.topper edited edge metadata.

LGTM. Let's see if the bots agree that no default constructor is needed.

This revision is now accepted and ready to land.Nov 8 2016, 9:54 PM
This revision was automatically updated to reflect the committed changes.