[PATCH] D58096: [LowerSwitch][AMDGPU] Do not handle impossible values
Roman Tereshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 14 17:57:36 PST 2019
rtereshin marked an inline comment as done.
rtereshin added inline comments.
Comment at: lib/Transforms/Utils/LowerSwitch.cpp:145
+ LazyValueInfo *LVI = &getAnalysis<LazyValueInfoWrapperPass>().getLVI();
> This is strange looking. I'm not sure what it really means to disable the dominator tree
According to http://llvm.org/doxygen/classllvm_1_1LazyValueInfo.html#a5b29ad30fb31c6df2a3cbcefef8ae613 it "Disables use of the DominatorTree within LVI." As far as I can tell, the idea is that if LVI is allowed to use DT, DT needs to be valid at every point. As the LowerSwitch pass creates and deletes BBs as it goes, it will mean updating DT along the way using a DomTreeUpdater. Please see
Author: Brian M. Rzycki <brzycki at gmail.com>
Date: Fri Feb 16 16:35:17 2018 +0000
[JumpThreading] PR36133 enable/disable DominatorTree for LVI analysis
The LazyValueInfo pass caches a copy of the DominatorTree when available.
Whenever there are pending DominatorTree updates within JumpThreading's
DeferredDominance object we cannot use the cached DT for LVI analysis.
This commit adds the new methods enableDT() and disableDT() to LVI.
JumpThreading also sets the appropriate usage model before calling LVI
Reviewers: sebpop, dberlin, kuhar
Reviewed by: sebpop, kuhar
Subscribers: uabelho, llvm-commits, aprantl, hiraditya, a.elovikov
Differential Revision: https://reviews.llvm.org/D42717
I did't have time to investigate much neither how profitable it is to have the LVI using DT vs not using DT, nor what the compile time cost of using the updater, but from a quick glance it appeared that LVI only uses DT to do `isValidAssumeForContext` for assumptions from the AssumptionCache, which is kinda useless at least for us as we rarely if ever have any `llvm.assume` intrinsic calls on icmp's in the IR. Not to mention, `isValidAssumeForContext` can handle some of the cases even w/o a DT.
CHANGES SINCE LAST ACTION
More information about the llvm-commits