[PATCH] D51581: [IndVars] Strengthen restricton in rewriteLoopExitValues

[Hongbin Zheng via Phabricator via llvm-commits]

etherzhhb requested changes to this revision.
etherzhhb added a comment.
This revision now requires changes to proceed.

> For some unclear reason rewriteLoopExitValues considers recalculation
>  after the loop profitable if it has some "soft uses" outside the loop (i.e. any
>  use other than call and return), even if we have proved that it has a user inside
>  the loop which we think will not be optimized away.

This also confuse me. Why it must be "soft uses" outside the loop that matter?
What I can guess is that, if there is no use outside, there is *no need* to rewrite the exit value.



================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:601-602
         //    optimized away.
         //  - no use outside of the loop can take advantage of hoisting the
         //    computation out of the loop
         if (ExitValue->getSCEVType()>=scMulExpr) {
----------------
looks like these comments describe the scenario of "SoftExternalUses", do we need to delete this as we deleted the corresponding code?


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:605-615
           for (auto *IB : Inst->users()) {
             Instruction *UseInstr = cast<Instruction>(IB);
             unsigned Opc = UseInstr->getOpcode();
-            if (L->contains(UseInstr)) {
-              if (Opc == Instruction::Call)
-                HasHardInternalUses = true;
-            } else {
-              if (Opc == Instruction::PHI) {
-                // Do not count the Phi as a use. LCSSA may have inserted
-                // plenty of trivial ones.
-                for (auto *PB : UseInstr->users()) {
-                  unsigned PhiOpc = cast<Instruction>(PB)->getOpcode();
-                  if (PhiOpc != Instruction::Call &&
-                      PhiOpc != Instruction::Ret) {
-                    HasSoftExternalUses = true;
-                    break;
-                  }
-                }
-                continue;
-              }
-              if (Opc != Instruction::Call && Opc != Instruction::Ret) {
-                HasSoftExternalUses = true;
-                break;
-              }
+            if (L->contains(UseInstr) && Opc == Instruction::Call) {
+              HasHardInternalUses = true;
+              break;
             }
----------------
we can further write:

```
// Skip if  Inst is used inside the loop in a way which can not be
//    optimized away.
if (llvm::any_of(Inst->users(), [L](auto *U) {
  return isa<CallInst>(UseInstr) && L->contains(UseInstr);
}))
  continue;
```

which is easier to read


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:607-608
             Instruction *UseInstr = cast<Instruction>(IB);
             unsigned Opc = UseInstr->getOpcode();
-            if (L->contains(UseInstr)) {
-              if (Opc == Instruction::Call)
-                HasHardInternalUses = true;
-            } else {
-              if (Opc == Instruction::PHI) {
-                // Do not count the Phi as a use. LCSSA may have inserted
-                // plenty of trivial ones.
-                for (auto *PB : UseInstr->users()) {
-                  unsigned PhiOpc = cast<Instruction>(PB)->getOpcode();
-                  if (PhiOpc != Instruction::Call &&
-                      PhiOpc != Instruction::Ret) {
-                    HasSoftExternalUses = true;
-                    break;
-                  }
-                }
-                continue;
-              }
-              if (Opc != Instruction::Call && Opc != Instruction::Ret) {
-                HasSoftExternalUses = true;
-                break;
-              }
+            if (L->contains(UseInstr) && Opc == Instruction::Call) {
+              HasHardInternalUses = true;
----------------
since opc is used only once now, maybe we could write:

```
if (isa<CallInst>(UseInstr) && L->contains(UseInstr)) {
```

Also, how about invoke?


https://reviews.llvm.org/D51581