[PATCH] D127603: [AArch64] isSeveralBitsExtractOpFromShr - match UBFM patterns with value tracking (RFC)

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1958
   // Value2 = AND Value, MaskImm
   // SRL Value2, ShiftImm
   //
----------------
RKSimon wrote:
> efriedma wrote:
> > The pattern we're looking for is SRL+AND.  The modified version of the function doesn't explicitly check for the "AND" operation at all.  I guess you can describe "AND" as "an operation with known zero bits in the result, and can be eliminated by SimplifyMultipleUseDemandedBits", but that seems like an awfully confusing description.
> > 
> > How does this change actually generalize the matching in practice?  Are we just looking for different AND masks, or can we actually match operations which aren't AND?
> > 
> > Maybe it would make the result easier to read if we extract the code that checks for the masking operation into a separate "MatchMaskOperation" function?
> Yes its most likely only ever going to match AND masks.
> 
> This was an experiment as a lot of recent SimplifyDemandedBits work (e.g. D125836) has managed to break the fragile bit extract patterns, and I was curious how general value tracking and demanded bits handling would work - although we're limited on how much we can manipulate the DAG at this late stage. I was curious what others thought, so I put up this RFC.
> 
> Maybe the aarch64 backend just needs additional canonicalization for some UBFX/SBFX patterns?
> 
> 
DAGCombine likes to manipulate basic arithmetic operations, so it's sort of hard to canonicalize them... but maybe some sort of canonicalization would be helpful.  We do already have AArch64TargetLowering::isDesirableToCommuteWithShift.

I haven't really thought about the ideal canonical form.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127603/new/

https://reviews.llvm.org/D127603