Switch DAGISel over to UniformityAnalysis, which was one of the last remaining users of the DivergenceAnalysis.
No explosions seen during internal testing so this looks like a smooth transition.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/include/llvm/Analysis/UniformityAnalysis.h | ||
---|---|---|
23 ↗ | (On Diff #504575) | This looks a bit disturbing to me. And it is squarely outside the limits of my understanding of C++ data types. I am even surprised that a "class UniformityInfo" declaration did not just work. What happens if you put the following in TargetLowering.h? extern template class GenericUniformityInfo<SSAContext>; using UniformityInfo = GenericUniformityInfo<SSAContext>; I would certainly recommend that compared to the current approach. |
llvm/include/llvm/Analysis/UniformityAnalysis.h | ||
---|---|---|
23 ↗ | (On Diff #504575) | class UniformityInfo doesn't work: [build] In file included from /home/pierre/work/trunk/llvm-project/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:16: [build] /home/pierre/work/trunk/llvm-project/llvm/include/llvm/Analysis/UniformityAnalysis.h:23:7: error: typedef redefinition with different types ('GenericUniformityInfo<llvm::SSAContext>' (aka 'GenericUniformityInfo<GenericSSAContext<llvm::Function>>') vs 'llvm::UniformityInfo') [build] using UniformityInfo = GenericUniformityInfo<SSAContext>; [build] ^ [build] /home/pierre/work/trunk/llvm-project/llvm/include/llvm/CodeGen/FunctionLoweringInfo.h:38:7: note: previous definition is here [build] class UniformityInfo; [build] ^ Your approach doesn't work unless the definition for SSAContext is visible I believe. A quick test with a forward declaration of both SSAContext and GenericUniformityInfo shows: [build] /home/pierre/work/trunk/llvm-project/llvm/include/llvm/ADT/GenericCycleInfo.h:235:12: error: field has incomplete type 'llvm::SSAContext' [build] ContextT Context; [build] ^ [build] /home/pierre/work/trunk/llvm-project/llvm/include/llvm/CodeGen/SelectionDAG.h:75:7: note: forward declaration of 'llvm::SSAContext' [build] class SSAContext; [build] ^ Note that the idiom I used isn't really a hack and doesn't change the nature of UniformityInfo (in any way that matters AFAIK), it just makes UniformityInfo forward-declarable using the struct keyword. Though I agree it's a bit weird to see so if we can find an alternative that doesn't involve adding new includes/5 lines of forward declarations for UniformityInfo then I'd love to use that instead. |
llvm/include/llvm/Analysis/UniformityAnalysis.h | ||
---|---|---|
23 ↗ | (On Diff #504575) | You can repeat something like this every time you want to forward-declare UniformityInfo: template<typename T> class GenericSSAContext; using SSAContext = GenericSSAContext<Function>; template<typename T> class GenericUniformityInfo; using UniformityInfo = GenericUniformityInfo<SSAContext>; Obviously this is a bit of a mouthful, but it's probably closer in spirit to what we've done in the past in LLVM. |
LGTM, with one nitpick.
llvm/include/llvm/CodeGen/TargetLowering.h | ||
---|---|---|
95 | SelectionDAG.h is included in this file ... so may be these declarations are redundant? |
Note: this seems to be causing a test failure with expensive checks enabled https://lab.llvm.org/buildbot/#/builders/16/builds/44820
I'm investigating and if I can't find a solution quick, I will revert.
SelectionDAG.h is included in this file ... so may be these declarations are redundant?