Details
- Reviewers
Joe_Nash dp Petar.Avramovic mbrkusanin
Diff Detail
Unit Tests
Time | Test | |
---|---|---|
60,030 ms | x64 debian > libFuzzer.libFuzzer::minimize_crash.test |
Event Timeline
Thanks for taking a look at this issue.
First, I am surprised there are not many more crashes without further modifications to codegen passes to account for the changed number of operands. For example
SIFoldOperand.cpp:693
The number of operands has changed, so I would expect removing the operands to break quite badly.
Actually, I think due to the way the operand lists for _e32 instructions are constructed, the modifiers are not taking effect.
If you dump the tablegen records, if the change was effective there should be a src0_modifiers operand on the V_MOV_B32_e32 instruction.
That said, I'm not convinced doing this properly would be worth the effort. Perhaps I should just close the issue.
@arsenm
@foad
llvm/test/MC/Disassembler/AMDGPU/gfx11_dasm_vop3_dpp8_from_vop1.txt | ||
---|---|---|
435 | It's curious that this results in sext(v2). It should print -v2. sext is not supported on v_mov_b32. Perhaps this is caused by the wrong operand class being set, or due to a bug in the inst printer. |
I guess the spec was changed between gfx9 and gfx10. On gfx10 and newer the docs say modifiers are supported on VOP1 as well as VOP3. But in terms of what our backend would make with just the above patch, it seems the modifiers would be on v_mov_b32_e64 but not v_mov_b32_e32. Hence my ask to check the tablegen records.
Ah yes, that makes a lot more sense. Sorry, I was not thinking clearly there.
The intention of this patch seems ok then to allow the modifiers on v_mov_b32_e64. Since modifiers are not supported on gfx9 and earlier, it would likely need the introduction of a new pseudo instruction for v_mov_b32, one for gfx10Plus that allows modifiers on VOP3.
But we make limited use of v_mov_b32_e64. So should more work be done to make use of it?
The potential utility is to do something like v_mov_b32 v0, -v1. But in a lot of cases this could be folded into the use of v0 by setting the neg modifier there.
It's curious that this results in sext(v2). It should print -v2. sext is not supported on v_mov_b32. Perhaps this is caused by the wrong operand class being set, or due to a bug in the inst printer.