[PATCH] D93093: [VE] Add logical mask intrinsic instructions

[Simon Moll via Phabricator via llvm-commits]

simoll added inline comments.


================
Comment at: llvm/lib/Target/VE/VEISelLowering.cpp:87-89
+    addRegisterClass(MVT::v256i1, &VE::VMRegClass);
+    addRegisterClass(MVT::v512i1, &VE::VM512RegClass);
   }
----------------
kaz7 wrote:
> simoll wrote:
> > Can we make this a separate patch?
> I condider that, but it's difficult to do so.  In order to do so, a regression test is required.  Making such a regression test requires additional implementation of vector mask register copy function.  I'm not sure what is required to implement that.
> 
> Probablly, I should ask them when you add above code.  I asked tests for V64 registers but forgot to ask tests of VM registers.
> 
> Anywa, this time I add new intrinsic instructions supporting vm512 and hit this bug.  You can see crashes by disabling above modifications and run new regression tests that this patch adds.  Then, you can see no crash after enabling above modifications and run the identical tests again.  Isn't that enough?
You can fix this in a separate patch without additional tests since it is obviously incorrect. I am asking you to do this because this patch adds new functionality first - we shouldn't bundle it up with bug fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93093