[llvm] r286289 - GlobalISel: allow CodeGen to fallback on VReg type/class issues.
David Jones via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 8 14:00:56 PST 2016
[+Tim]
On Tue, Nov 8, 2016 at 1:58 PM, David L. Jones <dlj at google.com> wrote:
> 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/3ac1713c/attachment.html>
More information about the llvm-commits
mailing list