Page MenuHomePhabricator

[AMDGPU][OpenMP] Remove optnone from outlined functions
AbandonedPublic

Authored by pdhaliwal on Oct 6 2021, 3:22 AM.

Details

Summary

This depends on D102107 and unblocks the failing amdgcn runtime
tests in the latter.

There seems to be some issue with function pointers handling
in the middle/back end. Removing optnone attribute makes the
optimizer to replace indirect call with direct call.

Diff Detail

Event Timeline

pdhaliwal created this revision.Oct 6 2021, 3:22 AM
pdhaliwal requested review of this revision.Oct 6 2021, 3:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 6 2021, 3:22 AM
JonChesterfield edited reviewers, added: tianshilei1992, ye-luo, grokos; removed: ggeorgakoudis.EditedOct 6 2021, 3:34 AM

Not pretty but unblocking D102107 is important. Could you write up your current understanding of what we're miscompiling for function calls as a comment on this diff (i.e. not in the code, just in the review discussion)? I'm guessing it's still attribute related but am a few days out of date.

I won't tag it as accepted because it's taking on debt in clang openmp to work around a bug in amdgpu middle/back end and I have a conflict of interest there. Hopefully an external person will accept.

pdhaliwal planned changes to this revision.Oct 6 2021, 4:05 AM

I don't have any concrete evidence but I have some doubt on presence of function pointers causing backend to behave improperly. Also, here removing optnone alone suffices to fix the issue.

pdhaliwal updated this revision to Diff 377502.Oct 6 2021, 4:47 AM

Only removing optnone.

pdhaliwal retitled this revision from [AMDGPU][OpenMP] Mark oulined functions always_inline to [AMDGPU][OpenMP] Remove optnone from outlined functions.Oct 6 2021, 4:49 AM
pdhaliwal edited the summary of this revision. (Show Details)

What if we track down the problem instead? This will simply pop up again in O0 user code, no?

Root cause would definitely be better but we don't have one at present and would like to unblock the linked diff. O0 is likely to have more problems than just this.

pdhaliwal abandoned this revision.Wed, Dec 1, 6:39 AM