Yep, OperandRange does not, but OpOperandVector does and it is used for DPS interface right now.

In D135348#3843656, @mravishankar wrote:

In D135348#3843577, @pifon2a wrote:

why is that needed ? Why cant you have an interface method that is just OpOperandVector getOutputOperands().

I was trying to avoid copying the OpOperands* all the time. In the pack/unpack ops, we can also move destination args to the and of the arg list. On the other hand, the same would need to be done for the already existing tensor.insert_slice. Which is also possible, but probably painful.

If we have $inits not at a fixed position, we have to construct SmallVector<OpOperand*> every time we call getOutputOperands. Also, does it mean that getNumOutputs would need to be expressed as getOutputOperands().size()? Would it make sense to have an interface method std::pair<unsigned, unsigned> getOutputsPositionsRange or smth like that that would return the interval of positions in the operand list that correspond to outputs? In that case, the users would need to override only this method.

@mravishankar

Yeah having an getOutputsPositionRange would work. AFAIK OperandRange doesnt own its data. The OpOperands are owned by the operation.

LGTM! Should we also add a release note or do we think this isn't enough of a compile-time performance improvement to warrant that?

Format the test.

In D135348#3843577, @pifon2a wrote:

why is that needed ? Why cant you have an interface method that is just OpOperandVector getOutputOperands().

I was trying to avoid copying the OpOperands* all the time. In the pack/unpack ops, we can also move destination args to the and of the arg list. On the other hand, the same would need to be done for the already existing tensor.insert_slice. Which is also possible, but probably painful.

If we have $inits not at a fixed position, we have to construct SmallVector<OpOperand*> every time we call getOutputOperands. Also, does it mean that getNumOutputs would need to be expressed as getOutputOperands().size()? Would it make sense to have an interface method std::pair<unsigned, unsigned> getOutputsPositionsRange or smth like that that would return the interval of positions in the operand list that correspond to outputs? In that case, the users would need to override only this method.

@mravishankar

Fix comments

Add TODO comment about sNaN. Thanks to @jcranmer-intel!

It's not clear to me if this instruction was ever selected. I don't see any ISel pattern or reference in tablegen or c++. In the synthesized tablgen defs, I see list<dag> Pattern = [(set i32:$vdst, (null_frag (i32 (VOP3Mods0 i32:$src0))))]; which I think doesn't get used because of the null_frag? Anyway, if it is being selected, do I need to update the pattern for the type change?

OperandRange does not own the data, does it? I was trying to avoid constructing SmallVector every time.

This is getting into the territory of actually being slower than a "mul", depending on the latency of "mul" and "add-with-shift" on the target CPU... we probably need CPU-specific modeling if you want to go this direction.

Fixes the failing AST unit test in Clang found by precommit CI.

Fixes https://github.com/llvm/llvm-project/issues/58230

why is that needed ? Why cant you have an interface method that is just OpOperandVector getOutputOperands().

Thanks for the review @aeubanks!

LGTM