[llvm] [NVPTX] Skip processing BasicBlocks with single unreachable instruction in `nvptx-lower-unreachable` pass. (PR #72641)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 15 13:45:20 PST 2024
================
@@ -138,7 +138,19 @@ bool NVPTXLowerUnreachable::runOnFunction(Function &F) {
InlineAsm *Exit = InlineAsm::get(ExitFTy, "exit;", "", true);
bool Changed = false;
- for (auto &BB : F)
+
+ // In scenarios where a BasicBlock contains only one unreachable instruction,
+ // the joint action of nvptx-isel and unreachable-mbb-elimination
+ // effectively optimizes the BasicBlock out. However, adding an exit
+ // command to such a BasicBlock, as suggested by this pass, preserves it
+ // within the Control Flow Graph (CFG), thereby negatively impacting size and
+ // performance. To counteract this undesirable consequence, we choose to
+ // refrain from processing BasicBlocks with just one unreachable instruction
+ // in this pass.
+
----------------
mmoadeli wrote:
> You're still proposing to trade off correctness for performance and I still strongly believe it's not something we want to do.
I disagree with you on this. The `exit` with an additional node in CGF is introduced by [this](https://reviews.llvm.org/D152789). If you build the test before that PR and with my change, you'll see they have identical ptx, whereas after that PR you'll see an additional node with `exit` in it.
If as claimed, this patch sacrifices correctness for performance, then before [this](https://reviews.llvm.org/D152789) we did not have correct behaviour in this situations.
I'd suggest comparing the ptx of three revision. 1) before [this](https://reviews.llvm.org/D152789), 2) after that, 3 after the fix introduced in this patch.
https://github.com/llvm/llvm-project/pull/72641
More information about the llvm-commits
mailing list