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

Michael Kuperstein michael.m.kuperstein at intel.com
Sun Jul 26 01:20:17 PDT 2015


mkuper added inline comments.

================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:295
@@ +294,3 @@
+      // See the comment in X86InstrInfo.td for more info.
+      if (!(OptForSize ||
+            MF->getFunction()->hasFnAttribute(Attribute::MinSize)))
----------------
hfinkel wrote:
> I think that you can remove the MinSize check. I don't have a strong opinion, but Clang always emits both for -Oz and emits OptimizeForSize for  -Os. Arguably, we should be checking for both everywhere, but we should make that change, if we make it, globally, not just in this one place.
> 
As a side note, there are already a couple of places that do the "or" check.

Admittedly, I wrote one of them myself (in X86CallFrameOpt), but there are at least two other significant places - both the initialization of AArch64DAGToDAGISel and the DAGCombiner constructors have:

```
ForCodeSize = F->hasFnAttribute(Attribute::OptimizeForSize) ||
              F->hasFnAttribute(Attribute::MinSize);
```

It would be great to make this consistent across the board, but I agree, that's a separate commit.


http://reviews.llvm.org/D11363







More information about the llvm-commits mailing list