-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[InitUndef] Enable the InitUndef pass on non-AMDGPU targets #108353
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
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,44 @@ | ||||||||||||
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5 | ||||||||||||
# RUN: llc -mtriple=aarch64-- -run-pass=init-undef -o - %s | FileCheck %s | ||||||||||||
|
||||||||||||
--- | | ||||||||||||
define dso_local i32 @test_stxp_undef(ptr %p, i64 %x) #0 { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need the IR section |
||||||||||||
%res = call i32 @llvm.aarch64.stxp(i64 undef, i64 %x, ptr %p) | ||||||||||||
ret i32 %res | ||||||||||||
} | ||||||||||||
|
||||||||||||
... | ||||||||||||
--- | ||||||||||||
name: test_stxp_undef | ||||||||||||
alignment: 4 | ||||||||||||
tracksRegLiveness: true | ||||||||||||
registers: | ||||||||||||
- { id: 0, class: gpr64common } | ||||||||||||
- { id: 1, class: gpr64 } | ||||||||||||
- { id: 2, class: gpr32 } | ||||||||||||
- { id: 3, class: gpr64 } | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
liveins: | ||||||||||||
- { reg: '$x0', virtual-reg: '%0' } | ||||||||||||
- { reg: '$x1', virtual-reg: '%1' } | ||||||||||||
body: | | ||||||||||||
bb.0 (%ir-block.0): | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
liveins: $x0, $x1 | ||||||||||||
|
||||||||||||
; CHECK-LABEL: name: test_stxp_undef | ||||||||||||
; CHECK: liveins: $x0, $x1 | ||||||||||||
; CHECK-NEXT: {{ $}} | ||||||||||||
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x1 | ||||||||||||
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64common = COPY $x0 | ||||||||||||
; CHECK-NEXT: [[DEF:%[0-9]+]]:gpr64 = IMPLICIT_DEF | ||||||||||||
; CHECK-NEXT: [[INIT_UNDEF:%[0-9]+]]:gpr64 = INIT_UNDEF | ||||||||||||
; CHECK-NEXT: early-clobber %2:gpr32 = STXPX killed [[INIT_UNDEF]], [[COPY]], [[COPY1]] :: (volatile store (s128) into %ir.p) | ||||||||||||
; CHECK-NEXT: $w0 = COPY %2 | ||||||||||||
; CHECK-NEXT: RET_ReallyLR implicit $w0 | ||||||||||||
%1:gpr64 = COPY $x1 | ||||||||||||
%0:gpr64common = COPY $x0 | ||||||||||||
%3:gpr64 = IMPLICIT_DEF | ||||||||||||
early-clobber %2:gpr32 = STXPX killed %3, %1, %0 :: (volatile store (s128) into %ir.p) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
$w0 = COPY %2 | ||||||||||||
RET_ReallyLR implicit $w0 | ||||||||||||
|
||||||||||||
... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could use an additional MIR test for something so specific
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pass itself already has MIR tests. I don't think adding another MIR test for this case would be useful, because we care about the end-to-end behavior (from the interaction of three different passes) here.
What would be quite nice is if the old code would emit an error, as it's hard to spot the issue otherwise. Currently it only generates an error when parsed back as inline assembly. Not sure whether there is a good way to share that validation, I'm not familiar with this part of the backend...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AArch64InstrInfo::verifyInstruction is the place for target-specific verification of an instruction... but currently it only gets called with -verify-machineinstrs. (It looks like AMDGPUAsmPrinter::emitInstruction calls verifyInstruction, but other targets currently don't do that.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see 3 places using run-pass=init-undef, all in RISCV. You could just dump the MIR before init-undef, extract the one instruction, and make sure the pass isn't a no-op on AArch64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, if all we want to test is "the pass does something", then a MIR test makes sense. I've added one.