[PATCH] D35594: [GISel]: ConstantFold operations when building MIR

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

Could you run clang-format on this patch? There's a few indentation and style nits in this patch. There's also a few in the neighbouring code which we should deal with separately

> Similarly we can constant fold binary operations such as Add/sub/or/and/xor/mul.

I don't see XOR in this patch

> In https://reviews.llvm.org/D35594#813925, @Gerolf wrote:
> 
>> Thanks for working on this! Please add comments and show off one or two examples that highlight the benefits of your extensions.
>>
>> Cheers
>> Gerolf
> 
> 
> 
> 
>> ! In https://reviews.llvm.org/D35594#814884, @aditya_nandakumar wrote:
> 
> This is helpful in cases where you try to build a generic operation on a constant.
>  For e.g.,
>  vreg1(s16) = G_CONSTANT 0
>  Builder.buildZExt(s32, vreg1) would result in
>  vreg1(s16) = G_CONSTANT 0
>  vreg2(s16) = G_ZEXT vreg1
> 
> Instead - we can constant fold operations and instead generate
>  vreg2(s32) = G_CONSTANT i32 0
> 
> Similarly we can constant fold binary operations such as Add/sub/or/and/xor/mul.
> 
> We also can constant fold intrinsics FABS/FLOG... (constant) -> constant.

I think Gerolf intended the comments and examples to be added to the patch itself.

In my opinion, the main thing this patch is missing is the test case. Is it possible to extend one (preferably a few) of the select-*.mir tests to cover a constant folding case?



================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:79-85
+  void buildSources(MachineInstrBuilder &MIB) { }
+  template<typename ... UseArgsTy>
+  void buildSources(MachineInstrBuilder &MIB, UseArgsTy &&... Args) {
+    unsigned It[] = {(getRegFromArg(Args))...};
+    for (const auto &i : It)
+      MIB.addUse(i);
+  }
----------------
This will need refreshing to account for the updates to one of your other patches.


================
Comment at: lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:185-186
+    case TargetOpcode::G_MUL: return C1 * C2;
+    case TargetOpcode::G_AND: return C1 & C2;
+    case TargetOpcode::G_OR: return C1 | C2;
+    }
----------------
XOR seems to be missing


Repository:
  rL LLVM

https://reviews.llvm.org/D35594