[llvm] r277001 - [GlobalISel] Remove types on selected insts instead of using LLT().

[Ahmed Bougacha via llvm-commits]

On Thu, Jul 28, 2016 at 9:58 AM, Ahmed Bougacha via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: ab
> Date: Thu Jul 28 11:58:27 2016
> New Revision: 277001
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277001&view=rev
> Log:
> [GlobalISel] Remove types on selected insts instead of using LLT().
>
> LLT() has a particular meaning: it's one invalid type. But we really
> want selected instructions to have no type whatsoever.
>
> Also verify that types don't linger after ISel, and enable the verifier
> on the AArch64 select test.
>
> Modified:
>     llvm/trunk/include/llvm/CodeGen/MachineInstr.h
>     llvm/trunk/lib/CodeGen/MachineInstr.cpp
>     llvm/trunk/lib/CodeGen/MachineVerifier.cpp
>     llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
>     llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineInstr.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineInstr.h?rev=277001&r1=277000&r2=277001&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineInstr.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineInstr.h Thu Jul 28 11:58:27 2016
> @@ -190,6 +190,7 @@ public:
>    void setType(LLT Ty, unsigned Idx = 0);
>    LLT getType(int unsigned = 0) const;
>    unsigned getNumTypes() const;
> +  void removeTypes();

Hey Tim, does this look good to you, or did you have something else in mind?

An alternative I considered was to have setDesc (or a new variant)
remove the types (maybe only if the opcode isn't generic?).

-Ahmed

>    /// Return true if MI is in a bundle (but not the first MI in a bundle).
>    ///
>
> Modified: llvm/trunk/lib/CodeGen/MachineInstr.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=277001&r1=277000&r2=277001&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineInstr.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineInstr.cpp Thu Jul 28 11:58:27 2016
> @@ -716,6 +716,8 @@ void MachineInstr::setType(LLT Ty, unsig
>
>  LLT MachineInstr::getType(unsigned Idx) const { return LLT{}; }
>
> +void MachineInstr::removeTypes() {}
> +
>  #else
>  unsigned MachineInstr::getNumTypes() const { return Tys.size(); }
>
> @@ -728,6 +730,10 @@ void MachineInstr::setType(LLT Ty, unsig
>  }
>
>  LLT MachineInstr::getType(unsigned Idx) const { return Tys[Idx]; }
> +
> +void MachineInstr::removeTypes() {
> +  Tys.clear();
> +}
>  #endif // LLVM_BUILD_GLOBAL_ISEL
>
>  /// RemoveRegOperandsFromUseLists - Unlink all of the register operands in
>
> Modified: llvm/trunk/lib/CodeGen/MachineVerifier.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineVerifier.cpp?rev=277001&r1=277000&r2=277001&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineVerifier.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineVerifier.cpp Thu Jul 28 11:58:27 2016
> @@ -879,6 +879,16 @@ void MachineVerifier::visitMachineInstrB
>      }
>    }
>
> +  // Check types.
> +  const unsigned NumTypes = MI->getNumTypes();
> +  if (isPreISelGenericOpcode(MCID.getOpcode())) {
> +    if (NumTypes == 0)
> +      report("Generic instruction must have a type", MI);
> +  } else {
> +    if (NumTypes != 0)
> +      report("Non-generic instruction cannot have a type", MI);
> +  }
> +
>    StringRef ErrorInfo;
>    if (!TII->verifyInstruction(*MI, ErrorInfo))
>      report(ErrorInfo.data(), MI);
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp?rev=277001&r1=277000&r2=277001&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp Thu Jul 28 11:58:27 2016
> @@ -150,7 +150,7 @@ bool AArch64InstructionSelector::select(
>
>      I.setDesc(TII.get(NewOpc));
>      // FIXME: Should the type be always reset in setDesc?
> -    I.setType(LLT());
> +    I.removeTypes();
>
>      // Now that we selected an opcode, we need to constrain the register
>      // operands to use appropriate classes.
>
> Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir?rev=277001&r1=277000&r2=277001&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir (original)
> +++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir Thu Jul 28 11:58:27 2016
> @@ -1,4 +1,4 @@
> -# RUN: llc -O0 -run-pass=instruction-select -global-isel %s -o - | FileCheck %s
> +# RUN: llc -O0 -run-pass=instruction-select -verify-machineinstrs -global-isel %s -o - | FileCheck %s
>  # REQUIRES: global-isel
>
>  # Test the instruction selector.
> @@ -6,7 +6,7 @@
>
>  --- |
>    target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
> -  target triple = "aarch64-apple-ios"
> +  target triple = "aarch64--"
>
>    define void @add_s32_gpr() { ret void }
>    define void @add_s64_gpr() { ret void }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits