Page MenuHomePhabricator

[AMDGPU][OpenMP] Use -amdgpu-fixed-function-abi
Needs RevisionPublic

Authored by pdhaliwal on Dec 1 2021, 5:25 AM.



This fixes issue of arguments clobbering when using indirect

Diff Detail

Unit TestsFailed

50 msx64 debian > LLVM.Transforms/LICM::scalar-promote-opaque-ptrs.ll
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -passes=licm -opaque-pointers -S /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/LICM/scalar-promote-opaque-ptrs.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/LICM/scalar-promote-opaque-ptrs.ll

Event Timeline

pdhaliwal created this revision.Dec 1 2021, 5:25 AM
pdhaliwal requested review of this revision.Dec 1 2021, 5:25 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
arsenm requested changes to this revision.Dec 1 2021, 6:04 AM

I have a patch to enable this by default and I do not want to spread uses of this flag around

This revision now requires changes to proceed.Dec 1 2021, 6:04 AM
JonChesterfield accepted this revision.Dec 1 2021, 6:36 AM

This would unblock a patch that has been waiting for us since May (D102107). Once your patches land and make the flag unnecessary we can take it back out. I asked for this because I need to get trunk to a less broken state asap to have a chance of working on other things and the currently broken ABI implementation makes a mess of that.

Fixed abi was set to true in D96340. At that point, indirect calls probably worked.
This was replaced with some dubious fine grain attribute handling in D99347, at which point indirect calls are probably broken. I think there was some more churn on function attributes around that time and expect ROCm and trunk have diverged somewhat at that point.

I therefore think they've been broken in edge cases since April.

Run a bunch of tests locally. This patch or something equivalent is a precondition on using the new device runtime on amdgpu, which we are very much out of time on (see D114890 which will break us as soon as it lands without this)