[PATCH] D38722: Added Remarks for Loop Versioning LICM Pass
Evgeny Astigeevich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 8 08:03:44 PST 2017
eastig added inline comments.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:210-212
+ bool isLegalForVersioning(OptimizationRemarkEmitter *ORE);
bool legalLoopStructure();
+ bool legalLoopInstructions(OptimizationRemarkEmitter *ORE);
----------------
What about to make ORE a class member?
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:397-399
+ return OptimizationRemarkMissed(DEBUG_TYPE, "IllegalLoopInst",
+ &Inst)
+ << " Illegal Loop Instruction";
----------------
Maybe it's better to use 'Unsafe' instead of 'Illegal'?
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:438-452
DEBUG(dbgs()
<< " Invariant load & store are less then defined threshold\n");
DEBUG(dbgs() << " Invariant loads & stores: "
<< ((InvariantCounter * 100) / LoadAndStoreCounter) << "%\n");
DEBUG(dbgs() << " Invariant loads & store threshold: "
<< InvariantThreshold << "%\n");
+ ORE->emit([&]() {
----------------
The message reported here is quite tricky. What I understand is: per cent of invariant memory operations in less then threshold per cent.
NV does not support FP values. Maybe it's better to add it instead of truncating 'static_cast<int>(InvariantThreshold)'. Diagnostics in this way do not provide actual information.
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:496-500
+ ORE->emit([&]() {
+ return OptimizationRemarkMissed(DEBUG_TYPE, "IllegalLoopInst",
+ CurLoop->getStartLoc(), CurLoop->getHeader())
+ << " Loop instructions not suitable for LoopVersioningLICM";
+ });
----------------
Do we need this remark here because remarks related to legality of instructions have already been emitted?
================
Comment at: lib/Transforms/Scalar/LoopVersioningLICM.cpp:507-511
+ ORE->emit([&]() {
+ return OptimizationRemarkMissed(DEBUG_TYPE, "IllegalLoopMemoryAccess",
+ CurLoop->getStartLoc(), CurLoop->getHeader())
+ << " Illegal Loop memory access";
+ });
----------------
Maybe legalLoopMemoryAccesses should emit remarks?
Repository:
rL LLVM
https://reviews.llvm.org/D38722
More information about the llvm-commits
mailing list