[llvm] r286289 - GlobalISel: allow CodeGen to fallback on VReg type/class issues.
David L. Jones via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 8 13:58:17 PST 2016
This breaks with -Werror, because there are a couple of variables that
aren't properly constrained by debugging.
It's not clear to me what the correct fix is, but I've just
sent https://reviews.llvm.org/D26421 with what looks to me to be the right
fix.
FAILED:
lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o
/usr/local/google/home/dlj/src/llvm/toolchain/bin/clang++
-DGTEST_HAS_RTTI=0 -DLLVM_BUILD_GLOBAL_ISEL -D_GNU_SOURCE
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-Ilib/CodeGen/GlobalISel
-I/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel -Iinclude
-I/usr/local/google/home/dlj/src/llvm/llvm/include -fPIC
-fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings
-Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long
-Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
-Werror -Werror=date-time -std=c++11 -fcolor-diagnostics
-ffunction-sections -fdata-sections -O3 -DNDEBUG -fno-exceptions
-fno-rtti -MD -MT
lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o
-MF
lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o.d
-o
lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/InstructionSelect.cpp.o
-c
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:105:20:
error: unused variable 'AfterIt' [-Werror,-Wunused-variable]
const auto AfterIt = std::next(MII);
^
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:141:27:
error: use of undeclared identifier 'MRI'
for (auto &VRegToType : MRI.getVRegToType()) {
^
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:143:16:
error: use of undeclared identifier 'MRI'
auto *RC = MRI.getRegClassOrNull(VReg);
^
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:144:16:
error: use of undeclared identifier 'MRI'
auto *MI = MRI.def_instr_begin(VReg) == MRI.def_instr_end()
^
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:144:45:
error: use of undeclared identifier 'MRI'
auto *MI = MRI.def_instr_begin(VReg) == MRI.def_instr_end()
^
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:146:24:
error: use of undeclared identifier 'MRI'
: &*MRI.def_instr_begin(VReg);
^
/usr/local/google/home/dlj/src/llvm/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:164:3:
error: use of undeclared identifier 'MRI'
MRI.getVRegToType().clear();
^
7 errors generated.
On Tuesday, November 8, 2016 at 12:48:53 PM UTC-8, Tim Northover via
llvm-commits 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.
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161108/796fc43d/attachment-0001.html>
More information about the llvm-commits
mailing list