[PATCH] D57812: [ARM] Add OptMinSize Subtarget feature

[Sam Parker via Phabricator via llvm-commits]

samparker marked 2 inline comments as done.
samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.h:570
 
-    bool shouldExpandShift(SelectionDAG &DAG, SDNode *N) const override {
-      if (DAG.getMachineFunction().getFunction().optForMinSize())
-        return false;
-      return true;
-    }
+    bool shouldExpandShift(SelectionDAG &DAG, SDNode *N) const override;
 
----------------
fhahn wrote:
> As the implementation is a one-liner, I guess we could keep it inline here.
This causes a bunch of confusing declaration issues...


================
Comment at: lib/Target/ARM/ARMSubtarget.h:715
   bool isThumb() const { return InThumbMode; }
+  void setOptForMinSize(bool MinSize) { OptMinSize = MinSize; }
+  bool optForMinSize() const { return OptMinSize; }
----------------
fhahn wrote:
> Do we anticipate STI users to change this setting? I think it would be better to set it directly in the constructor, if possible, to make sure STI.OptMinSize cannot diverge from the function attribute.
This doesn't work, I think because targets are cached in a map in TargetMachine so a new one isn't necessary constructed for each function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57812/new/

https://reviews.llvm.org/D57812