[IVDesc] Check loop-preheader for loop-legality when pass-remarks enabled #166310

ParkHanbum · 2025-11-04T05:51:46Z

When -pass-remarks=loop-vectorize is specified, the subsequent logic is executed to display detailed debug messages even if no PreHeader exists in the loop.

Therefore, an assert occurs when the getLoopPreHeader() function is called. This commit resolves that issue.

Fixed: #165377

llvmbot · 2025-11-04T05:52:20Z

@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: hanbeom (ParkHanbum)

Changes

When -pass-remarks=loop-vectorize is specified, the subsequent logic is executed to display detailed debug messages even if no PreHeader exists in the loop.

Therefore, an assert occurs when the getLoopPreHeader() function is called. This commit resolves that issue.

Fixed: #165377

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

2 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp (+8)
(added) llvm/test/Transforms/LoopVectorize/loop-legality-checks-remarks.ll (+33)

diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 8e09e6f8d4935..311cf4ecbe121 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -795,6 +795,14 @@ static bool canWidenCallReturnType(Type *Ty) {
 bool LoopVectorizationLegality::canVectorizeInstrs() {
   BasicBlock *Header = TheLoop->getHeader();
 
+  /// Generally, if `TheLoop` lacks a PreHeader, it won't reach this point.
+  /// However, when executed with remarks options like `-pass-remarks=vector`
+  /// it will reach here to output extra debug messages, so need to check.
+  /// Additionally, before reaching here, the debug messages would have
+  /// included `Loop doesn't have a legal pre-header`, so we omit that.
+  if (ORE->allowExtraAnalysis(DEBUG_TYPE) && !TheLoop->getLoopPreheader())
+    return false;
+
   // For each block in the loop.
   for (BasicBlock *BB : TheLoop->blocks()) {
     // Scan the instructions in the block and look for hazards.
diff --git a/llvm/test/Transforms/LoopVectorize/loop-legality-checks-remarks.ll b/llvm/test/Transforms/LoopVectorize/loop-legality-checks-remarks.ll
new file mode 100644
index 0000000000000..ec5dcacb0f765
--- /dev/null
+++ b/llvm/test/Transforms/LoopVectorize/loop-legality-checks-remarks.ll
@@ -0,0 +1,33 @@
+; RUN: opt < %s -passes=loop-vectorize -pass-remarks=loop-vectorize -debug -disable-output 2>&1 | FileCheck %s
+; REQUIRES: asserts
+
+; Make sure LV legal bails out when the loop doesn't have a legal pre-header.
+; CHECK-LABEL: 'not_exist_preheader'
+; CHECK: LV: Not vectorizing: Loop doesn't have a legal pre-header.
+define void @not_exist_preheader(ptr %currMB, i1 %arg, ptr %arg2) nounwind uwtable {
+entry:
+  br i1 %arg, label %start.exit, label %if.then.i
+
+if.then.i:
+  unreachable
+
+start.exit:
+  indirectbr ptr %arg2, [label %0, label %for.bodyprime]
+
+0:
+  unreachable
+
+for.bodyprime:
+  %i.057375 = phi i32 [0, %start.exit], [%1, %for.bodyprime]
+  %arrayidx8prime = getelementptr inbounds i32, ptr %currMB, i32 %i.057375
+  store i32 0, ptr %arrayidx8prime, align 4
+  %1 = add i32 %i.057375, 1
+  %cmp5prime = icmp slt i32 %1, 4
+  br i1 %cmp5prime, label %for.bodyprime, label %for.endprime
+
+for.endprime:
+  br label %for.body23prime
+
+for.body23prime:
+  br label %for.body23prime
+}

fhahn · 2025-11-06T11:19:00Z

llvm/test/Transforms/LoopVectorize/loop-legality-checks-remarks.ll

+entry:
+  br i1 %arg, label %start.exit, label %if.then.i
+
+if.then.i:
+  unreachable


I think you can remove those

fhahn · 2025-11-06T11:19:45Z

llvm/test/Transforms/LoopVectorize/loop-legality-checks-remarks.ll

+for.endprime:
+  br label %for.body23prime
+
+for.body23prime:
+  br label %for.body23prime


Can do something like below?

Suggested change

for.endprime:

br label %for.body23prime

for.body23prime:

br label %for.body23prime

exit:

ret void

Looks like the test still needs cleaning up?

fhahn · 2025-11-06T11:22:18Z

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

+  /// Generally, if `TheLoop` lacks a PreHeader, it won't reach this point.
+  /// However, when executed with remarks options like `-pass-remarks=vector`
+  /// it will reach here to output extra debug messages, so need to check.
+  /// Additionally, before reaching here, the debug messages would have
+  /// included `Loop doesn't have a legal pre-header`, so we omit that.
+  if (ORE->allowExtraAnalysis(DEBUG_TYPE) && !TheLoop->getLoopPreheader())
+    return false;


Checking here means we will miss out on any extra analysis for subsequent instructions.

We should be able to check in IVDescriptor and bail out if there's no preheader there, which would still allow continuing with the other checks.

You can add an unvectorizable instruction to the function, perhaps something like an unsupported call, for which we should get a missed remark ideally, even if there's no preheader.

I understand that you are asking me to modify the subcode so that no issues arise even if the PreHeader does not exist. Is that correct?

I understand that you are asking me to modify the subcode so that no issues arise even if the PreHeader does not exist. Is that correct?

Yep.

You can add an unvectorizable instruction to the function, perhaps something like an unsupported call, for which we should get a missed remark ideally, even if there's no preheader.

Looks like this is still pending?

github-actions · 2025-11-07T08:40:31Z

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

fhahn

Would be good to update the title to reflect the updates to the PR (now fixed in IVDescriptors)

fhahn · 2025-11-12T10:04:38Z

llvm/lib/Analysis/IVDescriptors.cpp

+  Value *RdxStart;
+  if (TheLoop->getLoopPreheader())
+    RdxStart = Phi->getIncomingValueForBlock(TheLoop->getLoopPreheader());


can we just return false here?

Suggested change

Value *RdxStart;

if (TheLoop->getLoopPreheader())

RdxStart = Phi->getIncomingValueForBlock(TheLoop->getLoopPreheader());

if (TheLoop->getLoopPreheader())

return false;

// Obtain the reduction start value from the value that comes from the loop

// preheader.

Value *RdxStart = Phi->getIncomingValueForBlock(TheLoop->getLoopPreheader());

sure, I'll update it

LLVM-Unit :: Analysis/./AnalysisTests/IVDescriptorsTest/FMaxRednIdentity LLVM-Unit :: Analysis/./AnalysisTests/IVDescriptorsTest/FMinRednIdentity Testing Time: 429.16s Total Discovered Tests: 113200 Skipped : 20 (0.02%) Unsupported : 1351 (1.19%) Passed : 111396 (98.41%) Expectedly Failed: 215 (0.19%) Failed : 218 (0.19%)

@fhahn After making the change, many failures occurred.

fhahn · 2025-11-12T10:05:26Z

llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp

+  /// Generally, if `TheLoop` lacks a PreHeader, it won't reach this point.
+  /// However, when executed with remarks options like `-pass-remarks=vector`
+  /// it will reach here to output extra debug messages, so need to check.
+  /// Additionally, before reaching here, the debug messages would have
+  /// included `Loop doesn't have a legal pre-header`, so we omit that.
+  if (ORE->allowExtraAnalysis(DEBUG_TYPE) && !TheLoop->getLoopPreheader())
+    return false;


I understand that you are asking me to modify the subcode so that no issues arise even if the PreHeader does not exist. Is that correct?

Yep.

You can add an unvectorizable instruction to the function, perhaps something like an unsupported call, for which we should get a missed remark ideally, even if there's no preheader.

Looks like this is still pending?

fhahn · 2025-11-12T10:05:40Z

llvm/test/Transforms/LoopVectorize/loop-legality-checks-remarks.ll

+for.endprime:
+  br label %for.body23prime
+
+for.body23prime:
+  br label %for.body23prime


Looks like the test still needs cleaning up?

…bled When `-pass-remarks=loop-vectorize` is specified, the subsequent logic is executed to display detailed debug messages even if no PreHeader exists in the loop. Therefore, an assert occurs when the `getLoopPreHeader()` function is called. This commit resolves that issue. Fixed: llvm#165377

llvmbot added vectorizers llvm:transforms labels Nov 4, 2025

ParkHanbum requested review from david-arm, fhahn and lukel97 November 4, 2025 14:57

fhahn reviewed Nov 6, 2025

View reviewed changes

ParkHanbum force-pushed the i165377 branch from 6fee527 to bab1bf1 Compare November 7, 2025 08:38

llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Nov 7, 2025

ParkHanbum force-pushed the i165377 branch from bab1bf1 to 05ffe9f Compare November 7, 2025 08:44

ParkHanbum requested a review from fhahn November 7, 2025 09:56

fhahn reviewed Nov 12, 2025

View reviewed changes

ParkHanbum force-pushed the i165377 branch from 05ffe9f to e7b19aa Compare November 13, 2025 06:57

ParkHanbum changed the title ~~[LV] Check loop-preheader for loop-legality when pass-remarks enabled~~ [IVDesc] Check loop-preheader for loop-legality when pass-remarks enabled Nov 13, 2025

ParkHanbum requested a review from fhahn November 14, 2025 14:48

[IVDesc] Check loop-preheader for loop-legality when pass-remarks enabled #166310

Are you sure you want to change the base?

[IVDesc] Check loop-preheader for loop-legality when pass-remarks enabled #166310

Conversation

ParkHanbum commented Nov 4, 2025

Uh oh!

llvmbot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fhahn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

llvmbot commented Nov 4, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading