[PATCH] D52904: [hot-cold-split] fix static analysis of cold regions
Brian Rzycki via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 9 10:48:33 PDT 2018
brzycki added a comment.
Hi @sebpop, I have a few comments inline regarding the function determining side effect status.
================
Comment at: llvm/lib/Transforms/IPO/HotColdSplitting.cpp:150
+ const TerminatorInst *I = BB.getTerminator();
+ if (isa<ReturnInst>(I) || isa<IndirectBrInst>(I) || isa<InvokeInst>(I))
+ return true;
----------------
Why is an Indirect branch considered a return? The instruction is required to branch to an address of a label within the current function.
I am also wondering about the EH TIs: `catchswitch`, `resume`, `catchret`, `cleanupret`. Should we just pessimize all EH code with:
```
if (I->isEHPad())
return true;
```
================
Comment at: llvm/lib/Transforms/IPO/HotColdSplitting.cpp:154
+ for (const Instruction &I : BB)
+ if (const CallInst *CI = dyn_cast<CallInst>(&I)) {
+ if (CI->hasFnAttr(Attribute::NoReturn))
----------------
Should we consider adding a per-instruction side-effect check?
```
if (I.mayHaveSideEffects())
return true;
```
https://reviews.llvm.org/D52904
More information about the llvm-commits
mailing list