[PATCH] D37592: [X86] Move matching of (and (srl/sra, C), (1<<C) - 1) to BEXTR/BEXTRI instruction to custom isel

[Ayman Musa via Phabricator via llvm-commits]

aymanmus added inline comments.


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2190
+  SDValue Tmp0, Tmp1, Tmp2, Tmp3, Tmp4;
+  if (tryFoldLoad(Node, Input, Tmp0, Tmp1, Tmp2, Tmp3, Tmp4)) {
+    SDValue Ops[] = { Tmp0, Tmp1, Tmp2, Tmp3, Tmp4, New, Input.getOperand(0) };
----------------
craig.topper wrote:
> aymanmus wrote:
> > Why do you manually handle the memory folding here instead of letting the regular mechanism take care of that?
> The regular mechanism is tablegen patterns and the isel table. But we're bypassing all of that here.
> 
> We could be lazy here and let the peephole pass that can also fold memory using the fold folding tables take care of it. But we normally try to fold as much as possible at isel.
Sorry if I'm being picky about that, but I can't understand the reason behind this.
In this case there is no special handling of memory folding (e.g. no special condition for applying/not applying the transformation), so why would we prefer to handle some of the cases here, some via tablegen patterns and the rest with the folding tables?

Wouldn't it be easier and more well-designed if the transformation would be completely controlled by one "owner" (lets say folding tables) and only exceptions are handled here or through tablegen patterns?


https://reviews.llvm.org/D37592