Skip to content

[M68k][NFC] Rename M68kOperand::Kind to KindTy - #112

Merged
cuviper merged 1 commit into
rust-lang:rustc/13.0-2021-08-08from
ricky26:rustc/13.0-2021-08-08
Aug 25, 2021
Merged

[M68k][NFC] Rename M68kOperand::Kind to KindTy#112
cuviper merged 1 commit into
rust-lang:rustc/13.0-2021-08-08from
ricky26:rustc/13.0-2021-08-08

Conversation

@ricky26

@ricky26 ricky26 commented Aug 25, 2021

Copy link
Copy Markdown

Rename the M68kOperand::Type enumeration to KindTy to avoid ambiguity
with the Kind field when referencing enumeration values e.g.
Kind::Value.

This works around a compilation error under GCC 5, where GCC won't
lookup enum class values if you have a similarly named field
(see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60994).

The error in question is:
M68kAsmParser.cpp:857:8: error: 'Kind' is not a class, namespace, or enumeration

Differential Revision: https://reviews.llvm.org/D108723


This is needed for the M68k backend to compile with the Ubuntu LTS builder (see rust-lang/rust#88321).

CC @glaubitz, @jrtc27, not sure who I need to include on this for review.

Rename the M68kOperand::Type enumeration to KindTy to avoid ambiguity
with the Kind field when referencing enumeration values e.g.
`Kind::Value`.

This works around a compilation error under GCC 5, where GCC won't
lookup enum class values if you have a similarly named field
(see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60994).

The error in question is:
`M68kAsmParser.cpp:857:8: error: 'Kind' is not a class, namespace, or enumeration`

Differential Revision: https://reviews.llvm.org/D108723

@cuviper cuviper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@cuviper
cuviper merged commit a93e47b into rust-lang:rustc/13.0-2021-08-08 Aug 25, 2021
@nikic

nikic commented Aug 26, 2021

Copy link
Copy Markdown

Could you please also file a bug blocking https://bugs.llvm.org/show_bug.cgi?id=release-13.0.0 to backport this to the upstream release branch?

@ricky26

ricky26 commented Aug 26, 2021

Copy link
Copy Markdown
Author

@nikic, okay, I've filed https://bugs.llvm.org/show_bug.cgi?id=51630.

vext01 pushed a commit to vext01/llvm-project that referenced this pull request Feb 7, 2024
alexcrichton pushed a commit to alexcrichton/llvm-project that referenced this pull request Aug 7, 2026
Swift async functions can miscompile on Windows ARM64 at `-O2`, when
there's enough register pressure that a local gets scavenged into the
callee-save area: the local ends up sharing an address with the saved
caller x29, so the epilogue restores a value the function has already
overwritten. swiftlang/swift#90920 has a reduced repro.

`assignCalleeSavedSpillSlots` creates the swift async context object
before the callee-save loop instead of inside it next to the FP slot.
MachineFrameInfo ends up with it above the frame record while the
prologue stores it below at FP-8, and the 8 byte disagreement leaves a
hole in the middle of the callee-save area. PEI's scavenger hands that
hole to the local. Only reproduces at -O2 and up since scavenging is
gated on the opt level.

```
	sub	sp, sp, rust-lang#112
	str	x19, [sp, rust-lang#16]                  // 8-byte Spill
	str	x21, [sp, rust-lang#24]                  // 8-byte Spill
	stp	x23, x24, [sp, rust-lang#32]             // 16-byte Folded Spill
	stp	x25, x26, [sp, rust-lang#48]             // 16-byte Folded Spill
	stp	x27, x28, [sp, rust-lang#64]             // 16-byte Folded Spill
	stp	x29, x30, [sp, rust-lang#88]             // 16-byte Folded Spill
	str	xzr, [sp, rust-lang#80]
	add	x29, sp, rust-lang#88
	...
	str	x7, [x29]                       // 8-byte Spill
	...
	ldr	x1, [x29]                       // 8-byte Reload
	ldp	x29, x30, [sp, rust-lang#88]             // 16-byte Folded Reload
```

This creates the object inside the loop so the two agree. The other
option was leaving the creation site alone and teaching MachineFrameInfo
about the expanded 24 byte FP/LR slot, but that puts the layout in two
places. Not sure which is preferred here, I don't know this code well.

This also asserts the saved FP object resolves to FP+0, since nothing
checks that today. Reverting the fix makes it fire on the same funclet.
`store-swift-async-context-clobber-live-reg.ll` already miscompiles with
`-regalloc=fast`, so this isn't Swift specific.
nikic pushed a commit that referenced this pull request Aug 21, 2026
Swift async functions can miscompile on Windows ARM64 at `-O2`, when
there's enough register pressure that a local gets scavenged into the
callee-save area: the local ends up sharing an address with the saved
caller x29, so the epilogue restores a value the function has already
overwritten. swiftlang/swift#90920 has a reduced repro.

`assignCalleeSavedSpillSlots` creates the swift async context object
before the callee-save loop instead of inside it next to the FP slot.
MachineFrameInfo ends up with it above the frame record while the
prologue stores it below at FP-8, and the 8 byte disagreement leaves a
hole in the middle of the callee-save area. PEI's scavenger hands that
hole to the local. Only reproduces at -O2 and up since scavenging is
gated on the opt level.

```
	sub	sp, sp, #112
	str	x19, [sp, #16]                  // 8-byte Spill
	str	x21, [sp, #24]                  // 8-byte Spill
	stp	x23, x24, [sp, #32]             // 16-byte Folded Spill
	stp	x25, x26, [sp, #48]             // 16-byte Folded Spill
	stp	x27, x28, [sp, #64]             // 16-byte Folded Spill
	stp	x29, x30, [sp, #88]             // 16-byte Folded Spill
	str	xzr, [sp, #80]
	add	x29, sp, #88
	...
	str	x7, [x29]                       // 8-byte Spill
	...
	ldr	x1, [x29]                       // 8-byte Reload
	ldp	x29, x30, [sp, #88]             // 16-byte Folded Reload
```

This creates the object inside the loop so the two agree. The other
option was leaving the creation site alone and teaching MachineFrameInfo
about the expanded 24 byte FP/LR slot, but that puts the layout in two
places. Not sure which is preferred here, I don't know this code well.

This also asserts the saved FP object resolves to FP+0, since nothing
checks that today. Reverting the fix makes it fire on the same funclet.
`store-swift-async-context-clobber-live-reg.ll` already miscompiles with
`-regalloc=fast`, so this isn't Swift specific.

(cherry picked from commit c358e8d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants