[PATCH] D76454: [ValueTracking] Fix usage of DataLayout::getTypeStoreSize()

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3500
 
-  const uint64_t Size = DL.getTypeStoreSize(V->getType());
+  const uint64_t Size = DL.getTypeStoreSize(V->getType()).getKnownMinSize();
   if (!Size)
----------------
I'm a little uncomfortable with the way getKnownMinSize() is being used here; it's correct, but I don't really like leaving around a variable deceptively named "Size" in a big function like this.

Maybe it would make sense to add a method isNonZero() to TypeSize?


================
Comment at: llvm/test/Transforms/MemCpyOpt/vscale.ll:5
+; Check that this test doesn't cause compier warning
+; warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.
+
----------------
We don't really need the text of the warning here...

I'm not really happy with the way the test here is exercising this codepath; if the code structure changes, it might end up covering nothing.  Can you add a positive test that we detect `<vscale x 4 x i32> zeroinitializer` as a "bytewise" value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76454