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

[NVPTX] Fix DwarfFrameBase construction #101000

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jul 29, 2024

The {0} here was initializing the first union member Register, rather than the union member used by CFA, which is Offset. Prior to #99263 this was harmless, but now they have different layout, leading to test failures on some platforms (at least i686 and s390x).

The `{0}` here was initializing the first union member `Register`,
rather than the union member used by CFA, which is `Location`.
Prior to llvm#99263 this was
harmless, but now they have different layout, leading to test
failures on some platforms.
@llvmbot
Copy link
Member

llvmbot commented Jul 29, 2024

@llvm/pr-subscribers-backend-nvptx

Author: Nikita Popov (nikic)

Changes

The {0} here was initializing the first union member Register, rather than the union member used by CFA, which is Offset. Prior to #99263 this was harmless, but now they have different layout, leading to test failures on some platforms (at least i686 and s390x).


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

1 Files Affected:

  • (modified) llvm/lib/Target/NVPTX/NVPTXFrameLowering.cpp (+4-1)
diff --git a/llvm/lib/Target/NVPTX/NVPTXFrameLowering.cpp b/llvm/lib/Target/NVPTX/NVPTXFrameLowering.cpp
index 10ae81e0460e3..9abe0e3186f20 100644
--- a/llvm/lib/Target/NVPTX/NVPTXFrameLowering.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXFrameLowering.cpp
@@ -93,5 +93,8 @@ MachineBasicBlock::iterator NVPTXFrameLowering::eliminateCallFramePseudoInstr(
 
 TargetFrameLowering::DwarfFrameBase
 NVPTXFrameLowering::getDwarfFrameBase(const MachineFunction &MF) const {
-  return {DwarfFrameBase::CFA, {0}};
+  DwarfFrameBase FrameBase;
+  FrameBase.Kind = DwarfFrameBase::CFA;
+  FrameBase.Location.Offset = 0;
+  return FrameBase;
 }

@wesleywiser
Copy link
Member

Thanks @nikic!

Does this also fix #100662?

@nikic
Copy link
Contributor Author

nikic commented Jul 29, 2024

I can't test on 32-bit ARM, but it should fix them there as well, yes.

@nikic nikic merged commit 842a332 into llvm:main Jul 29, 2024
9 checks passed
@nikic nikic deleted the nvptx-frame-base-fix branch July 29, 2024 20:46
@jakeegan
Copy link
Member

Thanks for the fix @nikic

@nikic
Copy link
Contributor Author

nikic commented Jul 30, 2024

/cherry-pick 842a332

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 30, 2024
The `{0}` here was initializing the first union member `Register`,
rather than the union member used by CFA, which is `Offset`. Prior to
llvm#99263 this was harmless, but
now they have different layout, leading to test failures on some
platforms (at least i686 and s390x).

(cherry picked from commit 842a332)
@llvmbot
Copy link
Member

llvmbot commented Jul 30, 2024

/pull-request #101145

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 30, 2024
The `{0}` here was initializing the first union member `Register`,
rather than the union member used by CFA, which is `Offset`. Prior to
llvm#99263 this was harmless, but
now they have different layout, leading to test failures on some
platforms (at least i686 and s390x).

(cherry picked from commit 842a332)
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
The `{0}` here was initializing the first union member `Register`,
rather than the union member used by CFA, which is `Offset`. Prior to
llvm#99263 this was harmless, but
now they have different layout, leading to test failures on some
platforms (at least i686 and s390x).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants