[RFC] Enforcing immediate operands for intrinsics

Hi,

I would like to solve the longstanding need for a way to indicate which parameters to an intrinsic are required to be immediates. It should be possible to declare in tablegen which parameters must be a trivial constant, or else the IR is invalid.

The verifier could then reject invalid intrinsic calls, so code handling the intrinsics doesn’t need to worry about invalid arguments. Currently any code that deals with such intrinsics needs to do type checks on the argument to avoid crashing on valid IR. This isn’t done particularly consistently (e.g. see r352904, or the follow-up r353097 for a recent example fix). From the codegen side, we do things like folding invalid intrinsic calls to undef during custom lowering, which is more boilerplate which shouldn’t be necessary.

It’s also necessary in a few some passes to know it’s illegal to replace an argument with a constant. llvm::canReplaceOperandWithVariable currently has to conservatively assume any intrinsic arguments can’t be touched.

I have 2 versions of partial implementations of this.

Adding an “immarg” attribute makes sense; thanks for working on this.

-Eli

Adding an "immarg" attribute makes sense; thanks for working on this.

+1

-Hal

-Eli

There is something similar going on in clang in SemaChecking.cpp. The difference is that each target can write its own verification code, which may check things like value ranges, for example.
The scheme you're proposing would invent a new attribute (a widespread change) that only implements partial checks (i.e. "is argument immediate or not").

Why not simply add a function to TTI that tells you whether a particular Value is a valid n-th argument to the given intrinsic?
Something like
   if (TTI.isValidArgument(IntOpc, Val, OpIdx))
     Int->setOperand(Val, OpIdx);

-Krzysztof

There is something similar going on in clang in SemaChecking.cpp. The difference is that each target can write its own verification code, which may check things like value ranges, for example.
The scheme you're proposing would invent a new attribute (a widespread change) that only implements partial checks (i.e. "is argument immediate or not”).

This is for the benefit of IR transform passes and codegen. I’m not attempting to diagnose frontend builtins or do anything for the end user. A backend can always truncate bits or whatever to fit the actual instruction constraints if necessary. It has to do something to not crash on the IR otherwise, which is part of the problem I’m trying to solve.

Why not simply add a function to TTI that tells you whether a particular Value is a valid n-th argument to the given intrinsic?
Something like
if (TTI.isValidArgument(IntOpc, Val, OpIdx))
   Int->setOperand(Val, OpIdx);

This is required for correctness, so TTI is not appropriate. Reasonably implementing this would still require adding something in TableGen (which then just brings you back to adding an attribute). I would also like to be able to rely on this for emission of G_INTRINISIC_* instructions in GlobalISel, so that intermediate illegal G_CONSTANT instructions can be avoided

-Matt

Ah, I thought you wanted to have the ability to verify an intrinsic in IR. Yeah, the attribute should be enough.

-Krzysztof

There’s a stale proposal with similar motivation here:
https://reviews.llvm.org/D23798

Setting the const-ness in the tablegen definition sounds better than that, but that patch may provide a starting point for intrinsics that have immarg potential.

+1