[llvm] r286289 - GlobalISel: allow CodeGen to fallback on VReg type/class issues.
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 10 14:51:17 PST 2016
> On Nov 8, 2016, at 12:39 PM, Tim Northover via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: tnorthover
> Date: Tue Nov 8 14:39:03 2016
> New Revision: 286289
>
> URL: http://llvm.org/viewvc/llvm-project?rev=286289&view=rev
> Log:
> GlobalISel: allow CodeGen to fallback on VReg type/class issues.
>
> After instruction selection we perform some checks on each VReg just before
> discarding the type information. These checks were assertions before, but that
> breaks the fallback path so this patch moves the logic into the main flow and
> reports a better error on failure.
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
> llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp
> llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h?rev=286289&r1=286288&r2=286289&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h Tue Nov 8 14:39:03 2016
> @@ -109,14 +109,6 @@ private:
> /// Map generic virtual registers to their actual size.
> mutable std::unique_ptr<VRegToTypeMap> VRegToType;
>
> - /// Accessor for VRegToType. This accessor should only be used
> - /// by global-isel related work.
> - VRegToTypeMap &getVRegToType() const {
> - if (!VRegToType)
> - VRegToType.reset(new VRegToTypeMap);
> - return *VRegToType.get();
> - }
> -
> /// Keep track of the physical registers that are live in to the function.
> /// Live in values are typically arguments in registers. LiveIn values are
> /// allowed to have virtual registers associated with them, stored in the
> @@ -642,6 +634,14 @@ public:
> ///
> unsigned createVirtualRegister(const TargetRegisterClass *RegClass);
>
> + /// Accessor for VRegToType. This accessor should only be used
> + /// by global-isel related work.
> + VRegToTypeMap &getVRegToType() const {
> + if (!VRegToType)
> + VRegToType.reset(new VRegToTypeMap);
> + return *VRegToType.get();
> + }
> +
> /// Get the low-level type of \p VReg or LLT{} if VReg is not a generic
> /// (target independent) virtual register.
> LLT getType(unsigned VReg) const;
> @@ -654,7 +654,8 @@ public:
> unsigned createGenericVirtualRegister(LLT Ty);
>
> /// Remove all types associated to virtual registers (after instruction
> - /// selection and constraining of all generic virtual registers).
> + /// selection and constraining of all generic virtual registers). Returns true
> + /// if the VReg mapping was consistent.
The comment is bogus; we do not return anything here.
> void clearVirtRegTypes();
>
> /// Creates a new virtual register that has no register class, register bank
>
> Modified: llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp?rev=286289&r1=286288&r2=286289&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp (original)
> +++ llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp Tue Nov 8 14:39:03 2016
> @@ -44,11 +44,13 @@ void InstructionSelect::getAnalysisUsage
> MachineFunctionPass::getAnalysisUsage(AU);
> }
>
> -static void reportSelectionError(const MachineInstr &MI, const Twine &Message) {
> - const MachineFunction &MF = *MI.getParent()->getParent();
> +static void reportSelectionError(const MachineInstr *MI, const Twine &Message) {
> + const MachineFunction &MF = *MI->getParent()->getParent();
> std::string ErrStorage;
> raw_string_ostream Err(ErrStorage);
> - Err << Message << ":\nIn function: " << MF.getName() << '\n' << MI << '\n';
> + Err << Message << ":\nIn function: " << MF.getName() << '\n';
> + if (MI)
> + Err << *MI << '\n';
> report_fatal_error(Err.str());
> }
>
> @@ -80,7 +82,7 @@ bool InstructionSelect::runOnMachineFunc
> for (const MachineBasicBlock &MBB : MF)
> for (const MachineInstr &MI : MBB)
> if (isPreISelGenericOpcode(MI.getOpcode()) && !MLI->isLegal(MI, MRI))
> - reportSelectionError(MI, "Instruction is not legal");
> + reportSelectionError(&MI, "Instruction is not legal");
>
> #endif
> // FIXME: We could introduce new blocks and will need to fix the outer loop.
> @@ -113,7 +115,10 @@ bool InstructionSelect::runOnMachineFunc
>
> if (!ISel->select(MI)) {
> if (TPC.isGlobalISelAbortEnabled())
> - reportSelectionError(MI, "Cannot select");
> + // FIXME: It would be nice to dump all inserted instructions. It's
> + // not
> + // obvious how, esp. considering select() can insert after MI.
> + reportSelectionError(&MI, "Cannot select");
> Failed = true;
> break;
> }
> @@ -129,21 +134,40 @@ bool InstructionSelect::runOnMachineFunc
> }
> }
>
> + // Now that selection is complete, there are no more generic vregs. Verify
> + // that the size of the now-constrained vreg is unchanged and that it has a
> + // register class.
> + for (auto &VRegToType : MRI.getVRegToType()) {
> + unsigned VReg = VRegToType.first;
> + auto *RC = MRI.getRegClassOrNull(VReg);
> + auto *MI = MRI.def_instr_begin(VReg) == MRI.def_instr_end()
> + ? nullptr
> + : &*MRI.def_instr_begin(VReg);
> + if (!RC) {
> + if (TPC.isGlobalISelAbortEnabled())
> + reportSelectionError(MI, "VReg as no regclass after selection");
> + Failed = true;
> + break;
> + }
> +
> + if (VRegToType.second.isValid() &&
> + VRegToType.second.getSizeInBits() > (RC->getSize() * 8)) {
> + if (TPC.isGlobalISelAbortEnabled())
> + reportSelectionError(
> + MI, "VReg has explicit size different from class size");
> + Failed = true;
> + break;
> + }
> + }
> +
> + MRI.getVRegToType().clear();
> +
> if (!TPC.isGlobalISelAbortEnabled() && (Failed || MF.size() == NumBlocks)) {
> MF.getProperties().set(MachineFunctionProperties::Property::FailedISel);
> return false;
> }
> assert(MF.size() == NumBlocks && "Inserting blocks is not supported yet");
>
> - // Now that selection is complete, there are no more generic vregs.
> - // FIXME: We're still discussing what to do with the vreg->size map:
> - // it's somewhat redundant (with the def MIs type size), but having to
> - // examine MIs is also awkward. Another alternative is to track the type on
> - // the vreg instead, but that's not ideal either, because it's saying that
> - // vregs have types, which they really don't. But then again, LLT is just
> - // a size and a "shape": it's probably the same information as regbank info.
> - MF.getRegInfo().clearVirtRegTypes();
> -
> // FIXME: Should we accurately track changes?
> return true;
> }
>
> Modified: llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp?rev=286289&r1=286288&r2=286289&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp Tue Nov 8 14:39:03 2016
> @@ -143,17 +143,6 @@ MachineRegisterInfo::createGenericVirtua
> }
>
> void MachineRegisterInfo::clearVirtRegTypes() {
> -#ifndef NDEBUG
> - // Verify that the size of the now-constrained vreg is unchanged.
> - for (auto &VRegToType : getVRegToType()) {
> - auto *RC = getRegClass(VRegToType.first);
> - if (VRegToType.second.isValid() &&
> - VRegToType.second.getSizeInBits() > (RC->getSize() * 8))
> - llvm_unreachable(
> - "Virtual register has explicit size different from its class size");
> - }
> -#endif
> -
> getVRegToType().clear();
> }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list