[PATCH] D135102: [AArch64] Compare BFI and ORR with left-shifted operand for OR instruction selection.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hmm. I had not considered am ORR with shift to be cheaper than a BFM before. From what I can tell it doesn't seem to be universal across all cpus, but does look like it will be faster or equal.

> A side question, is it a typical use case to convey metadata (e.g., op0 and op1 inside ORR op0, op1 contributes bits that doesn't overlap) in the SDNode class?

That sounds like it would usually be calculated with KnownBits, like in haveNoCommonBitsSet. Unfortunately post-isel the amount of information we can extract is much less than from the generic DAG nodes.

> [About the two/three options]

Is the motivating pattern just the one from the commit message, or any bfm that could be a shifted orr? aarch64-mi-peephole-opt is an option - we always run into problems implementing things there but if it is easier to write that is always an option. (The machine combiner too, if scheduling info is useful). Larger patterns might be more difficult though. The (existing) code in DAG2DAG doesn't feel like it scales super well. But equally like you say adding ISel nodes has downsides. What would this look like from GlobalISel? How much code would need to be added to make aarch64-mi-peephole-opt work?

Are the only regressions on uaddo_v4i1 and umulo_v4i1? I'm not against ignoring those, if they are just overflowing nodes on i1 types being awkwardly expanded and it doesn't come up in other places.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2835
+    //    and    x8, x23, #0x7f
+    //    ubfx   x9, x23, #8, #7
+    //    orr    x23, x8, x9, lsl #7
----------------
You say #7 here, but #8 elsewhere, including the test. I think 7 is correct for this example.


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

https://reviews.llvm.org/D135102