[PATCH] D139609: [AArch64][DAGCombiner] fold instruction BIC from ISD::AND
Dave Green via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 5 08:10:34 PST 2023
dmgreen added a comment.
> These method like `shouldFoldConstantShiftPairToMask` or `isDesirableToCommuteWithShift` only prevent DAG to transform a good pattern to bad pattern for us. But if the code is the bad pattern at first, we still can't optimize the case. So if DAGCombiner already has some canonicalization, I prefer to optimize the pattern after DAG combine. And I am not sure if we need add a TLI interface, what interface can we add to make the code general?
It appears to be this code that is transforming `not(and(x,C))` -> `or(not(x), not(C))`: https://github.com/llvm/llvm-project/blob/85d049a089d4a6a4a67145429ea5d8e155651138/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L8746
There would be advantages and disadvantages to both methods, but it seems like disabling that with a target hook, either universally or in specific cases should work and might be a little more general than this version. mvn_shiftedreg_from_and does get a little worse though, which can maybe be fixed in the DAG2DAG matchers? It could help other architectures if the not constant isn't as cheap as the original too.
If the reverse pattern is needed, it should be OK to add either as a DAG combine or in AArch64ISelLowering, so long as it does not fight with the existing combines.
I would recommend avoiding new AArch64ISD nodes, but if the alternative doesn't work then make sure we only start creating the node late - after the DAG has been legalized.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139609/new/
https://reviews.llvm.org/D139609
More information about the llvm-commits
mailing list