[WebAssembly] Fix MIR printing of reference types #113028
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When printing a memory operand in MIR, this line https://github.com/llvm/llvm-project/blob/d37bc32a65651e647148236ffb9728ea2e77eac3/llvm/lib/CodeGen/MachineOperand.cpp#L1247 calls this https://github.com/llvm/llvm-project/blob/d37bc32a65651e647148236ffb9728ea2e77eac3/llvm/include/llvm/Support/Alignment.h#L238 which assumes `Rhs` (the size in this case) is positive. But Wasm reference types' size is set to 0: https://github.com/llvm/llvm-project/blob/d37bc32a65651e647148236ffb9728ea2e77eac3/llvm/include/llvm/CodeGen/ValueTypes.td#L326-L328 It looks they didn't have a problem being printed in MIR until llvm#84751, which somehow removed the condition of `getSize() > 0` in that line. This revives the condition so that Wasm reference types will not crash the MIR printer.
|
@llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) ChangesWhen printing a memory operand in MIR, this line llvm-project/llvm/lib/CodeGen/MachineOperand.cpp Line 1247 in d37bc32 which assumes Rhs (the size in this case) is positive.
But Wasm reference types' size is set to 0: llvm-project/llvm/include/llvm/CodeGen/ValueTypes.td Lines 326 to 328 in d37bc32 It looks they didn't have a problem being printed in MIR until #84751, which somehow removed the condition of Full diff: https://github.com/llvm/llvm-project/pull/113028.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/MachineOperand.cpp b/llvm/lib/CodeGen/MachineOperand.cpp
index 89d32c3f005e00..c0e004555de959 100644
--- a/llvm/lib/CodeGen/MachineOperand.cpp
+++ b/llvm/lib/CodeGen/MachineOperand.cpp
@@ -1244,7 +1244,8 @@ void MachineMemOperand::print(raw_ostream &OS, ModuleSlotTracker &MST,
}
MachineOperand::printOperandOffset(OS, getOffset());
if (!getSize().hasValue() ||
- getAlign() != getSize().getValue().getKnownMinValue())
+ (!getSize().isZero() &&
+ getAlign() != getSize().getValue().getKnownMinValue()))
OS << ", align " << getAlign().value();
if (getAlign() != getBaseAlign())
OS << ", basealign " << getBaseAlign().value();
diff --git a/llvm/test/CodeGen/WebAssembly/externref-globalget.ll b/llvm/test/CodeGen/WebAssembly/externref-globalget.ll
index cdf98c42439f2d..79d7932486e226 100644
--- a/llvm/test/CodeGen/WebAssembly/externref-globalget.ll
+++ b/llvm/test/CodeGen/WebAssembly/externref-globalget.ll
@@ -1,4 +1,7 @@
; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types | FileCheck %s
+; Test for MIR printing of reference types in other address spaces. This should
+; not error out.
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types -print-after=finalize-isel | FileCheck %s
%externref = type ptr addrspace(10) ;; addrspace 10 is nonintegral
|
aheejin
changed the title
dschuff
approved these changes
Oct 22, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/7970 Here is the relevant piece of the build log for the reference |
|
@llvm-ci It doesn't seem to be relevant. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When printing a memory operand in MIR, this line
llvm-project/llvm/lib/CodeGen/MachineOperand.cpp
Line 1247 in d37bc32
llvm-project/llvm/include/llvm/Support/Alignment.h
Line 238 in d37bc32
Rhs(the size in this case) is positive.But Wasm reference types' size is set to 0:
llvm-project/llvm/include/llvm/CodeGen/ValueTypes.td
Lines 326 to 328 in d37bc32
getSize() > 0condition was added with the Wasm reference types support in 46667a1, and it looks it was removed in #84751. This revives the condition so that Wasm reference types will not crash the MIR printer.