[PATCH] D105159: [ARM] Fix incorrect assignment of Changed variable in MVEGatherScatterLowering::optimiseOffsets.

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 29 16:27:32 PDT 2021


craig.topper created this revision.
craig.topper added reviewers: dmgreen, SjoerdMeijer, samtebbs.
Herald added subscribers: danielkiss, hiraditya, kristof.beyls.
craig.topper requested review of this revision.
Herald added a project: LLVM.

I believe this Changed flag should be initialized to false,
otherwise the if (!Changed) is always dead. This doesn't
manifest in a functional issue because the PHINode checks will
fail if nothing changed. They are identical to the earlier
checks that must have already failed to get into this else block.

While there remove an else after return to reduce indentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105159

Files:
  llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp


Index: llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp
===================================================================
--- llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp
+++ llvm/lib/Target/ARM/MVEGatherScatterLowering.cpp
@@ -952,25 +952,23 @@
     Phi = cast<PHINode>(Offs->getOperand(1));
     OffsSecondOp = 0;
   } else {
-    bool Changed = true;
+    bool Changed = false;
     if (isa<Instruction>(Offs->getOperand(0)) &&
         L->contains(cast<Instruction>(Offs->getOperand(0))))
       Changed |= optimiseOffsets(Offs->getOperand(0), BB, LI);
     if (isa<Instruction>(Offs->getOperand(1)) &&
         L->contains(cast<Instruction>(Offs->getOperand(1))))
       Changed |= optimiseOffsets(Offs->getOperand(1), BB, LI);
-    if (!Changed) {
+    if (!Changed)
       return false;
+    if (isa<PHINode>(Offs->getOperand(0))) {
+      Phi = cast<PHINode>(Offs->getOperand(0));
+      OffsSecondOp = 1;
+    } else if (isa<PHINode>(Offs->getOperand(1))) {
+      Phi = cast<PHINode>(Offs->getOperand(1));
+      OffsSecondOp = 0;
     } else {
-      if (isa<PHINode>(Offs->getOperand(0))) {
-        Phi = cast<PHINode>(Offs->getOperand(0));
-        OffsSecondOp = 1;
-      } else if (isa<PHINode>(Offs->getOperand(1))) {
-        Phi = cast<PHINode>(Offs->getOperand(1));
-        OffsSecondOp = 0;
-      } else {
-        return false;
-      }
+      return false;
     }
   }
   // A phi node we want to perform this function on should be from the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105159.355395.patch
Type: text/x-patch
Size: 1473 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210629/0f3623b6/attachment.bin>


More information about the llvm-commits mailing list