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

[hfinkel at anl.gov]

hfinkel added a subscriber: hfinkel.

================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:289
@@ +288,3 @@
+    //
+    bool hoistImmediateForSize(SDNode *N) const {
+      uint32_t UseCount = 0;
----------------
This function does not really hoist anything, so maybe name this ShouldHoistImmediateForSize. Even that's confusing, however, because the constants are not actually immediates if they're hoisted, and you intend to enable this even when not optimizing for size. I think I prefer naming it:
  bool shouldEncodeConstantAsImmediate
or something like that.

================
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)))
----------------
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.



http://reviews.llvm.org/D11363