[PATCH] D35516: [GISel]: Split buildSources part of buildInstr separately

Daniel Sanders via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 24 12:29:30 PDT 2017


dsanders added a comment.

In https://reviews.llvm.org/D35516#819039, @aditya_nandakumar wrote:

> @dsanders - the main goal was to avoid the compiler warning of having to create a zero sized array.


You mean the `unsigned It[] = {};`? Ok, that makes sense. The subject line should reflect that though. E.g:

  [GISel] Avoid zero-length array when adding uses to instructions that lack them (e.g IMPLICIT_DEF).



================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:77-80
+  void addUseFromArg(MachineInstrBuilder &MIB, const MachineOperand &MO) {
+    MIB.add(MO);
+  }
 
----------------
This should be added by the patch that uses it.


================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:85
 
+  void buildSources(MachineInstrBuilder &MIB) { }
+  template<typename UseArgTy, typename ... UseArgsTy>
----------------
Just for consistency, should we name this addUsesFromArgs()?


================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:88-89
+  void buildSources(MachineInstrBuilder &MIB, UseArgTy &&Arg1, UseArgsTy &&... Args) {
+    addUseFromArg(MIB, Arg1);
+    buildSources(Args...);
+  }
----------------
I think this is missing some std::forward's


https://reviews.llvm.org/D35516





More information about the llvm-commits mailing list