Page MenuHomePhabricator

[X86][ABI] Change the alignment of f80 in 32-bit calling convention to meet with different data layout
ClosedPublic

Authored by pengfei on Nov 11 2021, 11:30 PM.

Diff Detail

Event Timeline

pengfei created this revision.Nov 11 2021, 11:30 PM
pengfei requested review of this revision.Nov 11 2021, 11:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2021, 11:30 PM

Should we be testing more target triples? I'm not clear how much variety there is on 32-bit targets.

llvm/test/CodeGen/X86/inline-asm-fpstack.ll
29

precommit the nounwind change so its obvious where the stack manip is coming from

pengfei updated this revision to Diff 386810.Nov 12 2021, 5:12 AM
pengfei marked an inline comment as done.

Rebased.

Should we be testing more target triples? I'm not clear how much variety there is on 32-bit targets.

I think it's enough. Currently we only have darwin that has different f80 alignment on 32-bit:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86TargetMachine.cpp#L130

Is there a bugzilla PR covering this?

llvm/test/CodeGen/X86/inline-asm-fpstack.ll
2

So maybe add a i386-linux-unknown test?

Is there a bugzilla PR covering this?

Probability not. I guess few people concerns 32-bit darwin now. I didn't mean to fix the bug. We just have the same requirment in our downstream code.

llvm/test/CodeGen/X86/inline-asm-fpstack.ll
2

I tried. Then I found the check doubled due to darwin and Linux have different inline asm comment. More importantly, the interesting info is buried by the large diff.
On the other hand, we have lots of lit tests involves f80 load/store. We have confidence given they are not changed :)

$ grep -r -E 'fldt|fstpt' llvm/test/CodeGen/X86/ | wc -l
994
$ grep -r -E 'fldt|fstpt' llvm/test/CodeGen/X86/ |cut -d ':' -f 1 |uniq | wc -l
58
RKSimon accepted this revision.Nov 12 2021, 6:43 AM

LGTM - thanks for checking

This revision is now accepted and ready to land.Nov 12 2021, 6:43 AM

Thanks Simon!

Minor comments for the title. It seems a typo "conversion -> convention".

pengfei retitled this revision from [X86][ABI] Change the alignment of f80 in 32-bit calling conversion to meet with different data layout to [X86][ABI] Change the alignment of f80 in 32-bit calling convention to meet with different data layout.Nov 12 2021, 5:01 PM

Thanks Yuanke!