[PATCH] D42191: [RFC] [TargetTransformInfo] Introduce isRegisterRich, it returns true if the target architecture is register-rich.

[Hongbin Zheng via Phabricator via llvm-commits]

etherzhhb added a comment.

In https://reviews.llvm.org/D42191#979308, @hfinkel wrote:

> In part, it looks like you're looking to modify heuristics that prevent increased spilling around calls. I can imagine that, in general, architectures with lots of registers suffer from less of that, but that's really a statement about the ABI of the call, not the architecture itself.


The original motivation is about hoisting the GEP, in general I want to GVN the gep like:

bb0:
idx0 = i + 2
g0 = gep A, idx0
br bb2

bb1:
idx1 = i + 2
g1 = gep A, idx1
br bb2

bb2:
g = phi [g0, bb0]; [g1, bb1]

In this example, g1 and g2 is not GVNed because our concern about the register pressure (or address mode?).

> Also, are you targeting FPGAs?

Yes

> Maybe this is better phrased in terms of architectures that don't have physical registers (we already have backend targets that have only virtual registers), because lacking physical registers, I can imagine register pressure being much less concerning. 
>  Arbitrary hoisting might not be great for FPGA HLS either, because of increased routing pressure and the like,

Not sure the place and route pressure, but too much registers may still have a negative impact on FPGAs.

> but if we imagine that the backend can sink when useful, then maybe we're just making a statement about being reliant on that?

Yes.

I am thinking maybe some of the LLVM IR passes could provide a mode that run without the concern of physical constraints like register pressure, and simplify the dataflow of the LLVM IR as much as possible.

I am trying to identify the actual physical constraints that prevents simplification like GVN from happening, and try to make these constraints more explicit and provide a hook to relax these constraints if possible.



================
Comment at: lib/Transforms/Scalar/GVNHoist.cpp:1045
         if (!allOperandsAvailable(Repl, DestBB)) {
           // When HoistingGeps there is nothing more we can do to make the
           // operands available: just continue.
----------------
hfinkel wrote:
> Update this comment?
Yes


================
Comment at: lib/Transforms/Scalar/GVNHoist.cpp:1101
         // deeper may increase the register pressure and compilation time.
         if (MaxDepthInBB != -1 && InstructionNb++ >= MaxDepthInBB)
           break;
----------------
hfinkel wrote:
> Would you want to modify this check too?
yes


================
Comment at: lib/Transforms/Scalar/GVNHoist.cpp:1127
+        } else if (TTI.isRegisterRich() || !isa<GetElementPtrInst>(&I1))
           // Do not hoist scalars past calls that may write to memory because
           // that could result in spills later. geps are handled separately.
----------------
hfinkel wrote:
> Is this comment accurate? I don't see why this is talking about calls here when calls are handled in the block above.
I guess it is about the address mode since it is related to GEP


Repository:
  rL LLVM

https://reviews.llvm.org/D42191