G_INTRINSIC and convergent operations

Code Generation
1

Hi all,

GMIR currently provides two opcodes for intrinsics:

 1219 // Intrinsic without side effects.
 1220 def G_INTRINSIC : GenericInstruction {
 1221   let OutOperandList = (outs);
 1222   let InOperandList = (ins unknown:$intrin, variable_ops);
 1223   let hasSideEffects = false;
 1224
 1225   // Conservatively assume this is convergent. If there turnes out to
 1226   // be a need, there should be separate convergent intrinsic opcodes.
 1227   let isConvergent = 1;
 1228 }
 1229
 1230 // Intrinsic with side effects.
 1231 def G_INTRINSIC_W_SIDE_EFFECTS : GenericInstruction {
 1232   let OutOperandList = (outs);
 1233   let InOperandList = (ins unknown:$intrin, variable_ops);
 1234   let hasSideEffects = true;
 1235   let mayLoad = true;
 1236   let mayStore = true;
 1237
 1238   // Conservatively assume this is convergent. If there turnes out to
 1239   // be a need, there should be separate convergent intrinsic opcodes.
 1240   let isConvergent = true;
 1241 }

I am working on distinguishing between intrinsics based on whether they are convergent. But I don’t fully understand why there should be separate opcodes for that. Each intrinsic already declares whether it is convergent or not. Then, rather than a new generic opcode, why not have an additional MICD that describes the intrinsic? This can be used to decide if a particular occurrence is convergent.

Was this already considered as an option? Am I missing something there?

If we go with separate generic opcodes, we may have something like this:

G_INTRINSIC
G_INTRINSIC_W_SIDE_EFFECTS
G_INTRINSIC_NOTCONVERGENT
G_INTRINSIC_NOTCONVERGENT_W_SIDE_EFFECTS

This recognizes the fact that intrinsics are already assumed to be convergent by default.

Alternatively, we could have:

G_INTRINSIC
G_INTRINSIC_W_SIDE_EFFECTS
G_INTRINSIC_CONVERGENT
G_INTRINSIC_CONVERGENT_W_SIDE_EFFECTS

This would mean that we remove the isConvergent property from the existing opcodes and then conservatively update all existing occurrences to G_INTRINSIC_CONVERGENT.

But where does this end? Will another property further multiply the number of opcodes?

Also separately, there are other opcodes like G_INTRINSIC_TRUNC, which correspond to @llvm intrinsic functions. They are not really intrinsics, right? So a hypothetical MachineInstr::isIntrinsic() should return false for such opcodes?

Sameer.

2

Hi @ssahasra

How would you have an additional MICD?
If the MICD is not obtained through the regular path (MachineInstr::getDesc), we would need to do some plumbing to get the right information everywhere.

No, I think the idea was that we treat the intrinsics conservatively. No deep thinking went through this AFAIR. We essentially replicated what SDISel was doing and added isConvergent because SDISel had no capabilities to move things around.

Depending on what you’re after, you always have the option of lowering your intrinsic as soon as you want (and hence get the right MCID).

Cheers,
-Quentin

3

I think this was named confusingly in an effort to avoid confusion with fptrunc. I think the INTRINSIC should be dropped by the name.

I think we should just go with the set of 4 opcodes (with the second set of names). We could also add a convergent flag to MIFlag, but there are dangers of flag dropping. Plus, in this case you want to add an additional operand to the instructions so that acts more like a distinct opcode.

1 Like
4

I don’t have an authoritative solution. But the general idea is that G_INTRINSIC will have it’s own MCID as usual, and a secondary MCID that corresponds to the IntrinsicID MachineOperand. The secondary MCID is consulted only for properties of the called intrinsic like isConvergent(). This will require a new field in MachineInstr to hold the second MCID, and it can be assigned in MachineIRBuilder::buildIntrinsic().

But that’s just general open thoughts. For now, I intend to continue implementing the two new G_INTRINSIC_CONVERGENT opcodes.

That would be nice. If we really do want to differentiate, then perhaps G_LLVM_TRUNC would be a less confusing name, and more accurately describe the correspondence to @llvm intrinsics.

5

Adding the _CONVERGENT opcodes sounds fine to me.

+1 on renaming of G_INTRINSIC_TRUNC and so on.