[PATCH] D111530: [TargetLowering] Optimize expanded SRL/SHL fed into SETCC ne/eq 0

[Filipp Zhinkin via Phabricator via llvm-commits]

fzhinkin added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:3497
+  // has a height of 8, so it should be enough to cover most practical cases.
+  constexpr unsigned MaxTreeHeight = 8;
+  if (!Scan(N0, MaxTreeHeight))
----------------
RKSimon wrote:
> Why was i4096 of particular interest?
> 
> We have a default depth limit we use for this kind of thing: SelectionDAG::MaxRecursionDepth = 6, also, normally we increment a depth to that value instead of decrementing a height.
Nothing really interesting about i4096, just mentioned it to depict that max height of 8 is large enough for any type someone will use in actual code.

I'll change `8` to `SelectionDAG::MaxRecursionDepth` and decrement to increment. Thanks!


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:3517
+  // Reduce all values using OR.
+  for (size_t Index = 0; Index + 1 < Result.size(); Index += 2) {
+    SDValue NewOr = DAG.getNode(ISD::OR, DL, N0.getValueType(), Result[Index],
----------------
RKSimon wrote:
> Can't you avoid pushing results by just OR'ing the results once?
> ```
> SDValue Reduction = Result[0];
> for (size_t I = 1, E = Results.size(); I < E; ++I)
>   Reduction = DAG.getNode(ISD::OR, DL, N0.getValueType(), Reduction, Result[I]);
> ```
I'm pushing ORs back to the results list to generate balanced tree (and shorten critical path's length). And it pays off at least for ARM:

```
; llc -O3 -mtriple=armv7 test.ll
define i1 @opt_setcc_shl_ne_zero_i128(i128 %a) nounwind {
   %shl = shl i128 %a, 17
   %cmp = icmp ne i128 %shl, 0
   ret i1 %cmp
}
```

Code generated using current implementation:
```
opt_setcc_shl_ne_zero_i128:
@ %bb.0:
	orr	r2, r2, r3, lsl #17
	orr	r0, r1, r0
	orrs	r0, r0, r2
	movwne	r0, #1
	bx	lr
```

Code generated using implementation OR'ing in place:
```
opt_setcc_shl_ne_zero_i128:
@ %bb.0:
	orr	r0, r1, r0
	orr	r0, r0, r2
	orr	r0, r0, r3, lsl #17
	cmp	r0, #0
	movwne	r0, #1
	bx	lr
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111530