[PATCH] D11363: Allow merging of immediates within a basic block for code size savings and reduced footprint.

[Zia Ansari]

zansari updated this revision to Diff 31245.
zansari added a comment.

OK, back from vacation.. Thanks for the additional reviews and comments. Some updates:

- Hal: I agree with your comment regarding the naming for the querying function that doesn't actually hoist anything. I wasn't 100% comfortable with the name you suggested, since it was a bit confusing (to me, at least). I went with "shouldAvoidImmediateInstFormsForSize", if that works for you.

- Hal: I also removed the minsize check, per your recommendation.

- Quentin / Sanjay:

Here are my thoughts regarding ConstantHoisting.cpp and http://reviews.llvm.org/D5544...

http://reviews.llvm.org/D5544 is overkill, in my opinion. It's a lot of code that costs extra computation and does something that can be easily done in existing places.

Regarding ConstantHoisting.cpp, I did look into this at first and my earliest prototype was done through there. I'd still like to explore global (cross-BB) constant hoisting in the future, but I chose to go with my current proposed changes for a couple of main reasons:

1. ConstantHoisting.cpp can make the constants more opaque, preventing isel from pulling them, back in, but this is phase dependent. How about constants introduced after hoisting? DAG creation will always pull out constants, and it seems more natural to properly select the right instructions after this transformation, given our heuristics.

2. Perhaps more importantly : Pulling out all common constants in ConstantHoisting.cpp might affect other downstream target-independent optimizations on the IL that look for constants as part of the instructions. It's easier to optimize with subsumed immediates at the IL level. My changes may have the same effect in the CG, but they occur later in isel, and are a little more natural for something that's more target-specific (i.e. dealing with instruction encoding).

Those two reasons, along with wanting to tackle this the safest/easiest way (without rocking the boat too much) were my primary motivations for the route that I took.

Hope this helps clear up some of the confusion.

Thanks,
Zia.


http://reviews.llvm.org/D11363

Files:
  lib/Target/X86/X86ISelDAGToDAG.cpp
  lib/Target/X86/X86InstrArithmetic.td
  lib/Target/X86/X86InstrInfo.td
  test/CodeGen/X86/immediate_merging.ll
  test/CodeGen/X86/remat-invalid-liveness.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11363.31245.patch
Type: text/x-patch
Size: 13524 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150803/b9c71ea5/attachment.bin>