[PATCH] D136523: [MSAN] Add handleCountZeroes for vector versions of ctlz and cttz.

[Kevin Athey via Phabricator via llvm-commits]

kda added a comment.

In D136523#3880730 <https://reviews.llvm.org/D136523#3880730>, @eugenis wrote:

> LGTM, but I'd merge the two changes. I don't really see the point of submitting a test for the broken behavior first.

The test does not demonstrate "broken" behavior but rather shows what is generated before my change.
I will keep them separate as I would like to show how my code changes what is generated.



================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2982
+    // If zero poison is requested, mix in with the shadow
+    Value *BoolZeroPoison = ConstantInt::getFalse(BoolShadow->getType());
+    assert(isa<Constant>(I.getOperand(1)));
----------------
eugenis wrote:
> This line seems unnecessary.
Oh.  Again, I rearranged the code and forgot about this dangling item.  I originally had the OR outside the conditional, thus I needed a BoolZeroPoison that was passive.

Removed.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2974
+  void handleCountZeroes(IntrinsicInst &I) {
+    // if not vector operation, then just use default
+    Type::TypeID InputTypeID = I.getArgOperand(0)->getType()->getTypeID();
----------------
eugenis wrote:
> isn't this implementation strictly better than default? It handles the second operand correctly.
Agreed.  I added this comment (conditional) prior to adding handling of second parameter.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2976
+    Type::TypeID InputTypeID = I.getArgOperand(0)->getType()->getTypeID();
+    if (InputTypeID != Type::FixedVectorTyID) {
+      if (!handleUnknownIntrinsic(I))
----------------
eugenis wrote:
> Is there a problem with scalable vectors? You could just use Type::isVectorTy() otherwise.
dropped (see previous comment).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136523