This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Disable optimizeEndCf at -O0
ClosedPublic

Authored by cdevadas on Jan 7 2022, 9:32 AM.

Diff Detail

Event Timeline

cdevadas created this revision.Jan 7 2022, 9:32 AM
cdevadas requested review of this revision.Jan 7 2022, 9:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 7 2022, 9:32 AM
rampitec added inline comments.Jan 7 2022, 11:34 AM
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
813

You can just check it where optimizeEndCf is called below.

814

There is skipFunction() method, it will check optnone attribute. Just checking -O0 does not check this.

arsenm added inline comments.Jan 7 2022, 11:55 AM
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
814

skipFunction wouldn't be appropriate because this is still a lowering pass we must run

rampitec added inline comments.Jan 7 2022, 11:57 AM
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
814

It still can be checked to skip optimization part of it only. Isn't it?

arsenm added inline comments.Jan 7 2022, 11:58 AM
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
814

You can also just directly check optnone

cdevadas added inline comments.Jan 7 2022, 5:42 PM
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
813

There are two reasons why I did that:

  1. There is already a check inside the function to skip it based on the command-line option. I want to unify all skips.
  2. At any point in the future, if there are multiple such call-sites, it would be good to have the skip inserted at its entry.
814

Will do.

I think this is an extreme interpretation of optnone. This is a minor optimization which happens as part of lowering. The fact we do this as a separate step is just an artifact of how we happen to lower control flow. It's not strictly true that no optimizations occur at -O0, especially if they are cheap and provide benefit

I think this is an extreme interpretation of optnone. This is a minor optimization which happens as part of lowering. The fact we do this as a separate step is just an artifact of how we happen to lower control flow. It's not strictly true that no optimizations occur at -O0, especially if they are cheap and provide benefit

I assume it is good to have a way to disable it with -O0 just for bug bisecting.

I think this is an extreme interpretation of optnone. This is a minor optimization which happens as part of lowering. The fact we do this as a separate step is just an artifact of how we happen to lower control flow. It's not strictly true that no optimizations occur at -O0, especially if they are cheap and provide benefit

I assume it is good to have a way to disable it with -O0 just for bug bisecting.

I agree with Stas. This optimization should be skipped for -O0. We can avoid the optnone check if it doesn't fit this context. The existing optLevel check in the patch would do then.

I think this is an extreme interpretation of optnone. This is a minor optimization which happens as part of lowering. The fact we do this as a separate step is just an artifact of how we happen to lower control flow. It's not strictly true that no optimizations occur at -O0, especially if they are cheap and provide benefit

I assume it is good to have a way to disable it with -O0 just for bug bisecting.

The bisecting will only work if we can disable the entire pass, which we can't do

rampitec accepted this revision.Jan 17 2022, 10:06 AM
This revision is now accepted and ready to land.Jan 17 2022, 10:06 AM
This revision was automatically updated to reflect the committed changes.