This will do the combine in cases that should fold, but don't
now. e.g. we're relying on the CSEMIRBuilder's incomplete constant
folding. For instance it doesn't handle FP operations or vectors (and
we don't have separate constant folding combines either to catch
them).
Details
Diff Detail
Event Timeline
llvm/lib/CodeGen/GlobalISel/Utils.cpp | ||
---|---|---|
1157 | That is only on ConstantInt, not ConstantFP |
llvm/lib/CodeGen/GlobalISel/Utils.cpp | ||
---|---|---|
1157 | No, it's on Constant. |
llvm/lib/CodeGen/GlobalISel/Utils.cpp | ||
---|---|---|
1157 | Oh, that just dispatches on ConstantFP and does the same thing |
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | ||
---|---|---|
3142 | In the match function, you found the G_SELECT using getOpcodeDef, which walks through COPY instructions. If that walked through a COPY, I think this will give you the wrong instruction. | |
llvm/lib/CodeGen/GlobalISel/Utils.cpp | ||
1135 | Opcode is only used here? |
Would it be hard to teach CSEMIRBuilder to handle the missing cases so that you can match top-down from the G_SELECT? It would be nice to avoid maintaining a list of constant-foldable opcodes if possible.
CSEMIRBuilder won't help here, and patterns apply bottom up. Really these are all constant foldable, we just don't handle a handful of them. I don't think it's that important and this could instead use the full list of binary opcodes
Rebase.
Since I posted this, we now have a second version of isConstantOrConstantVector which should probably be merged
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | ||
---|---|---|
3142 | It's theoretically wrong but due to the copy combines, this was unobservably broken |
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | ||
---|---|---|
3142 | Would adding a testcase which only enables fold_binop_into_select make it possible to test? |
I think this looks fine
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | ||
---|---|---|
3094 | can pull out Select->getOpcode() into a variable |
For instance it doesn't handle FP operations or vectors
Is this still true? I think we do have at least some vector folds now?
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | ||
---|---|---|
3148 | It does cope with it, we don't actually care about the source operand types at this point. This is already covered in the tests |
There are some vectors (plus some FP ops are handled in another one of these patches I forgot about)
"truncation truncation"?