[PATCH] D141471: [AArch64][SVE] Fix crash for DestructiveBinaryComm zero merging

[Allen zhong via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:561
     // prefixed_zeroing_mov for DestructiveBinary.
-    assert((DOPRegIsUnique || AArch64::DestructiveBinary == DType) &&
+    assert((DOPRegIsUnique || (AArch64::DestructiveBinary == DType ||
+                               AArch64::DestructiveBinaryComm == DType)) &&
----------------
paulwalker-arm wrote:
> This being an || chain already in brackets I doubt you need these inner brackets.
Apply your comment, thanks


================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:561
     // prefixed_zeroing_mov for DestructiveBinary.
-    assert((DOPRegIsUnique || AArch64::DestructiveBinary == DType) &&
+    assert((DOPRegIsUnique || (AArch64::DestructiveBinary == DType ||
+                               AArch64::DestructiveBinaryComm == DType)) &&
----------------
paulwalker-arm wrote:
> paulwalker-arm wrote:
> > Allen wrote:
> > > paulwalker-arm wrote:
> > > > This being an || chain already in brackets I doubt you need these inner brackets.
> > > Apply your comment, thanks
> > I think you've misunderstood my comment and have also introduced a bug.  I meant there's no need for any new brackets i.e. you can write `(DOPRegIsUnique || AArch64::DestructiveBinary == DType || AArch64::DestructiveBinaryComm == DType) && ...`
> > 
> > Also with the new update you've incorrectly replaced the `DestructiveBinaryComm` comparison within an assignment of `DType`.
> I've pushed a fix.
Oh, sorry. Thanks very much


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141471