[llvm] [X86, SimplifyCFG] Support hoisting load/store with conditional faulting (Part I) (PR #96878)
Phoebe Wang via llvm-commits
llvm-commits at lists.llvm.org
Sun Nov 17 07:26:42 PST 2024
phoebewang wrote:
> > > I think that this isn't something SimplifyCFG should be handling.
> >
> >
> > This is completely opposite to what I thought. Can you tell me the reasoning behind your idea?
>
> Probably worth considering this comment above the transform:
>
> https://github.com/llvm/llvm-project/blob/95ce78b742b2965f3a4a42115a96a330d779a98d/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L3153-L3162
>
> I believe the machine passes it refers to are EarlyIfConversion and IfConversion. I do think it would be worthwhile to consider whether this transform isn't better handled there, as these passes can actually properly cost-model such transforms, unlike SimplifyCFG.
>
> I'm not really familiar with these passes, but from a quick look at EarlyIfConversion, it seems to support two strategies, one which only does pure speculation (so no store speculation) and the other that does predication. For X86, what we want is a bit in the middle, in the sense that we only predicate load/store instructions and speculate the rest.
>
> It's probably not so hard to extend that code with an additional policy for this case (or maybe extend the speculation policy to allow conditional load/store predication), and may give you better mileage than trying to do it in SimplifyCFG.
Thanks @nikic for the suggestion! Sorry for the late response, I just got some time to investigate it.
I think HexagonEarlyIfConversion does approximate work as you described, but in a separate pass. It's true we can evaluate the cost more precisely, but there are also two drawbacks compared with SimplifyCFG solution.
- Machine pass is more conservative in speculating load/store instructions;
- X86 introduced a new predicate compare instruction, which is selected during ISel and relied on flattened CFG. It's not easy to reconstruct back in machine pass;
As the comments mentioned, it's also beneficial to instcombine DAG combiner, so I think it's better to use SimplifyCFG here.
https://github.com/llvm/llvm-project/pull/96878
More information about the llvm-commits
mailing list