[PATCH] D20262: [DSE]Split memset when the memset is small enough to be lowered to stores

[Quentin Colombet via llvm-commits]

qcolombet added a comment.

Couple additional comments.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:465
@@ -462,1 +464,3 @@
+  unsigned getMaxStoresPerMemset(Function *F) const;
+
   /// \return The expected cost of arithmetic ops, such as mul, xor, fsub, etc.
----------------
Is nullptr permitted for F?
If not, then use a reference.

================
Comment at: include/llvm/Target/TargetLowering.h:966
@@ +965,3 @@
+  /// lowering memory operation intrinsics.
+  bool shouldLowerMemFuncForSize(const Function *F) const ;
+
----------------
Same question, nullptr OK or not?

================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:1648
@@ +1647,3 @@
+  if (getTargetMachine().getTargetTriple().isOSDarwin())
+    return F->optForMinSize();
+  return F->optForSize();
----------------
What the comment says is correct. However, I wonder if we want to have this difference here.
I would have expected something simpler like return F->optForSize() || F->optForMinSize() for all OS.

================
Comment at: test/Transforms/DeadStoreElimination/SplitMemintrinsic.ll:7
@@ +6,3 @@
+define void @test_32_8to15(i64* nocapture %P, i64 %n64) {
+; CHECK:  %0 = getelementptr inbounds i8, i8* %Base, i64 16
+; CHECK:  call void @llvm.memset.p0i8.i64(i8* %Base, i8 0, i64 8, i32 8, i1 false)
----------------
Don’t hard code %0.
Use a regexp.


http://reviews.llvm.org/D20262