This is an archive of the discontinued LLVM Phabricator instance.

[X86] Don't avoid folding multiple use sign extended 8-bit immediate into instructions under optsize.
ClosedPublic

Authored by craig.topper on Mar 18 2019, 4:58 PM.

Details

Summary

Under optsize we try to avoid folding immediates into instructions under optsize. But if the immediate is 16-bits or 32 bits, but can be encoded as an 8-bit immediate we don't save enough from disabling the folding unless the immediate has enough uses to make up for the size of the move which is either 3 bytes or 5 bytes since there are no sign extended 8-bit moves. We would also save something if the immediate was a live out of the basic block and thus a move was unavoidable, but that would require a more advanced heuristic than just counting uses.

Note we only avoid folding multiple use immediates into the patterns that use X86ISD::ADD/SUB/XOR/OR/AND/CMP/ADC/SBB nodes and not the more common ISD::ADD/SUB/XOR/OR/AND nodes.

Diff Detail

Event Timeline

craig.topper created this revision.Mar 18 2019, 4:58 PM
craig.topper marked an inline comment as done.Mar 18 2019, 5:03 PM
craig.topper added inline comments.
test/CodeGen/X86/immediate_merging64.ll
14

This is weird. Once the compare selected. The use count on the -1 constant should have dropped and allowed it fold into the store. Unless isel selected the store before the cmp. I think this demonstrates a larger flaw in the use count check as part of instruction selection. The use count of constants can be influenced by instruction selection order.

Seems good, but let Simon have a look too in case there's some uarch concern that I'm not aware of.

test/CodeGen/X86/immediate_merging.ll
14–15

Partly stale comment. Add a line for 8-bit exception.

RKSimon accepted this revision.Mar 21 2019, 8:48 AM

LGTM

test/CodeGen/X86/immediate_merging64.ll
14

Please can you raise a bug on this - we're collecting a lot of immediate constant bugs at the moment.......

This revision is now accepted and ready to land.Mar 21 2019, 8:48 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2019, 10:41 AM