Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][indvars] Remove unused code in WidenIV::widenLoopCompare #73506

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

wenju-he
Copy link
Contributor

No description provided.

Copy link

github-actions bot commented Nov 27, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@wenju-he wenju-he marked this pull request as ready for review November 29, 2023 23:47
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 29, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Wenju He (wenju-he)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/73506.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/SimplifyIndVar.cpp (-4)
diff --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
index a23ac41acaa58aa..a1d09fd73c29129 100644
--- a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
@@ -1493,10 +1493,6 @@ bool WidenIV::widenLoopCompare(WidenIV::NarrowIVDefUse DU) {
   assert(CastWidth <= IVWidth && "Unexpected width while widening compare.");
 
   // Widen the compare instruction.
-  auto *InsertPt = getInsertPointForUses(DU.NarrowUse, DU.NarrowDef, DT, LI);
-  if (!InsertPt)
-    return false;
-  IRBuilder<> Builder(InsertPt);
   DU.NarrowUse->replaceUsesOfWith(DU.NarrowDef, DU.WideDef);
 
   // Widen the other operand of the compare, if necessary.

@wenju-he
Copy link
Contributor Author

CI clang-format error isn't related to this pr:

Running: git-clang-format --diff f[21](https://github.com/llvm/llvm-project/actions/runs/7004455642/job/19052227219?pr=73506#step:10:22)a70f9fe21539f70212ba[23](https://github.com/llvm/llvm-project/actions/runs/7004455642/job/19052227219?pr=73506#step:10:24)46c3db54f4d9980 c9808bf052d7ae13c3a8fb7678bdd[25](https://github.com/llvm/llvm-project/actions/runs/7004455642/job/19052227219?pr=73506#step:10:26)73a00deb9 -- llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
error: clang-format exited with code 1
diff --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
index a1d09fd73c..47ecc89055 100644
--- a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
@@ -1[37](https://github.com/llvm/llvm-project/actions/runs/7004455642/job/19052227219?pr=73506#step:10:38)9,10 +1379,10 @@ WidenIV::getExtendedOperandRecurrence(WidenIV::NarrowIVDefUse DU) {
   ExtendKind ExtKind = getExtendKind(DU.NarrowDef);
   if (ExtKind == ExtendKind::Sign && OBO->hasNoSignedWrap())
     ExtendOperExpr = SE->getSignExtendExpr(
-      SE->getSCEV(DU.NarrowUse->getOperand(ExtendOperIdx)), WideType);
+        SE->getSCEV(DU.NarrowUse->getOperand(ExtendOperIdx)), WideType);
   else if (ExtKind == ExtendKind::Zero && OBO->hasNoUnsignedWrap())
     ExtendOperExpr = SE->getZeroExtendExpr(
-      SE->getSCEV(DU.NarrowUse->getOperand(ExtendOperIdx)), WideType);
+        SE->getSCEV(DU.NarrowUse->getOperand(ExtendOperIdx)), WideType);
   else
     return {nullptr, ExtendKind::Unknown};
 

error: some formatters failed: clang-format
Error: Process completed with exit code 1.

@boomanaiden154
Copy link
Contributor

That clang-format diff seems to be coming from 7c93452. Not sure why that's getting picked up.

I've opened an issue at #73873 as this shouldn't be happening. I'm guessing if you rebase against a recent main, the immediate issue will be fixed while investigation/work is done on fixing the underlying issue in the CI.

@wenju-he
Copy link
Contributor Author

That clang-format diff seems to be coming from 7c93452. Not sure why that's getting picked up.

I've opened an issue at #73873 as this shouldn't be happening. I'm guessing if you rebase against a recent main, the immediate issue will be fixed while investigation/work is done on fixing the underlying issue in the CI.

thank you @boomanaiden154 for the analysis. clang-format check passed after rebase.

However, there is new error:

<html>
<body>
<!--StartFragment-->
$ git clean -ffxdq
--
  | # Fetch and checkout pull request head from GitHub
  | $ git fetch -v --prune -- origin refs/pull/73506/head
  | POST git-upload-pack (401 bytes)
  | POST git-upload-pack (943 bytes)
  | POST git-upload-pack (gzip 2543 to 1316 bytes)
  | POST git-upload-pack (gzip 5743 to 2922 bytes)
  | POST git-upload-pack (gzip 12143 to 6112 bytes)
  | remote: Enumerating objects: 7, done.
  | remote: Counting objects: 100% (6/6), done.
  | remote: Total 7 (delta 6), reused 6 (delta 6), pack-reused 1
  | Unpacking objects: 100% (7/7), 7.25 KiB \| 200.00 KiB/s, done.
  | From https://github.com/llvm/llvm-project
  | * branch                      refs/pull/73506/head -> FETCH_HEAD
  | # FETCH_HEAD is now `c9808bf052d7ae13c3a8fb7678bdd2573a00deb9`
  | $ git checkout -f b77f90718a325c0ead0e041aed31f3ee7cf8cc56
  | fatal: reference is not a tree: b77f90718a325c0ead0e041aed31f3ee7cf8cc56
  | ⚠️ Warning: Checkout failed! checking out commit "b77f90718a325c0ead0e041aed31f3ee7cf8cc56": exit status 128 (Attempt 1/3 Retrying in 2s)
  | $ git clean -ffxdq
  | # Fetch and checkout pull request head from GitHub
  | $ git fetch -v --prune -- origin refs/pull/73506/head
  | POST git-upload-pack (401 bytes)
  | From https://github.com/llvm/llvm-project
  | * branch                      refs/pull/73506/head -> FETCH_HEAD
  | # FETCH_HEAD is now `c9808bf052d7ae13c3a8fb7678bdd2573a00deb9`
  | $ git checkout -f b77f90718a325c0ead0e041aed31f3ee7cf8cc56
  | fatal: reference is not a tree: b77f90718a325c0ead0e041aed31f3ee7cf8cc56
  | ⚠️ Warning: Checkout failed! checking out commit "b77f90718a325c0ead0e041aed31f3ee7cf8cc56": exit status 128 (Attempt 2/3 Retrying in 2s)
  | $ git clean -ffxdq
  | # Fetch and checkout pull request head from GitHub
  | $ git fetch -v --prune -- origin refs/pull/73506/head
  | POST git-upload-pack (401 bytes)
  | From https://github.com/llvm/llvm-project
  | * branch                      refs/pull/73506/head -> FETCH_HEAD
  | # FETCH_HEAD is now `c9808bf052d7ae13c3a8fb7678bdd2573a00deb9`
  | $ git checkout -f b77f90718a325c0ead0e041aed31f3ee7cf8cc56
  | fatal: reference is not a tree: b77f90718a325c0ead0e041aed31f3ee7cf8cc56
  | ⚠️ Warning: Checkout failed! checking out commit "b77f90718a325c0ead0e041aed31f3ee7cf8cc56": exit status 128 (Attempt 3/3)
  | 🚨 Error: checking out commit "b77f90718a325c0ead0e041aed31f3ee7cf8cc56": exit status 128

<!--EndFragment-->
</body>
</html>

@boomanaiden154 Do you have any idea about this error? Thank you

@boomanaiden154
Copy link
Contributor

@boomanaiden154 Do you have any idea about this error? Thank you

No, I don't. As long as your tests pass locally, I wouldn't worry too much about the Buildkite failure. In a couple days the infrastructure for that should be getting migrated to Github actions now that Phabricator CI support is going away, so there's not going to be a lot of effort put into fixing these sorts of things on the Buildkite site.

@wenju-he
Copy link
Contributor Author

@boomanaiden154 Do you have any idea about this error? Thank you

No, I don't. As long as your tests pass locally, I wouldn't worry too much about the Buildkite failure. In a couple days the infrastructure for that should be getting migrated to Github actions now that Phabricator CI support is going away, so there's not going to be a lot of effort put into fixing these sorts of things on the Buildkite site.

Thank you.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yubingex007-a11y yubingex007-a11y merged commit 2441e23 into llvm:main Dec 1, 2023
3 checks passed
@wenju-he wenju-he deleted the indvars-dead-code branch December 1, 2023 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants