[llvm] r276875 - [GlobalISel] Introduce an instruction selector.

[Ahmed Bougacha via llvm-commits]

On Thu, Jul 28, 2016 at 8:13 AM, Ahmed Bougacha
<ahmed.bougacha at gmail.com> wrote:
> On Wed, Jul 27, 2016 at 5:41 PM, Tom Stellard <tom at stellard.net> wrote:
>> On Wed, Jul 27, 2016 at 02:31:56PM -0000, Ahmed Bougacha via llvm-commits wrote:
>>> Author: ab
>>> Date: Wed Jul 27 09:31:55 2016
>>> New Revision: 276875
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=276875&view=rev
>>> Log:
>>> [GlobalISel] Introduce an instruction selector.
>>>
>>> And implement it for AArch64, supporting x/w ADD/OR.
>>>
>>> Differential Revision: https://reviews.llvm.org/D22373
>>>
>>
>> <snip>
>>
>>>
>>> Added: llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp?rev=276875&view=auto
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp (added)
>>> +++ llvm/trunk/lib/CodeGen/GlobalISel/InstructionSelect.cpp Wed Jul 27 09:31:55 2016
>>> @@ -0,0 +1,99 @@
>>> +//===- llvm/CodeGen/GlobalISel/InstructionSelect.cpp - InstructionSelect ---==//
>>> +//
>>> +//                     The LLVM Compiler Infrastructure
>>> +//
>>> +// This file is distributed under the University of Illinois Open Source
>>> +// License. See LICENSE.TXT for details.
>>> +//
>>> +//===----------------------------------------------------------------------===//
>>> +/// \file
>>> +/// This file implements the InstructionSelect class.
>>> +//===----------------------------------------------------------------------===//
>>> +
>>> +#include "llvm/CodeGen/GlobalISel/InstructionSelect.h"
>>> +#include "llvm/ADT/PostOrderIterator.h"
>>> +#include "llvm/ADT/Twine.h"
>>> +#include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
>>> +#include "llvm/CodeGen/MachineRegisterInfo.h"
>>> +#include "llvm/IR/Function.h"
>>> +#include "llvm/Support/CommandLine.h"
>>> +#include "llvm/Support/Debug.h"
>>> +#include "llvm/Target/TargetSubtargetInfo.h"
>>> +
>>> +#define DEBUG_TYPE "instruction-select"
>>> +
>>> +using namespace llvm;
>>> +
>>> +char InstructionSelect::ID = 0;
>>> +INITIALIZE_PASS(InstructionSelect, DEBUG_TYPE,
>>> +                "Select target instructions out of generic instructions",
>>> +                false, false);
>>> +
>>> +InstructionSelect::InstructionSelect() : MachineFunctionPass(ID) {
>>> +  initializeInstructionSelectPass(*PassRegistry::getPassRegistry());
>>> +}
>>> +
>>> +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';
>>> +  report_fatal_error(Err.str());
>>> +}
>>> +
>>> +bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {
>>> +  DEBUG(dbgs() << "Selecting function: " << MF.getName() << '\n');
>>> +
>>> +  const InstructionSelector *ISel = MF.getSubtarget().getInstructionSelector();
>>> +  assert(ISel && "Cannot work without InstructionSelector");
>>> +
>>> +  // FIXME: freezeReservedRegs is now done in IRTranslator, but there are many
>>> +  // other MF/MFI fields we need to initialize.
>>> +
>>> +#ifndef NDEBUG
>>> +  // FIXME: We could introduce new blocks and will need to fix the outer loop.
>>> +  // Until then, keep track of the number of blocks to assert that we don't.
>>> +  const size_t NumBlocks = MF.size();
>>> +#endif
>>> +
>>> +  for (MachineBasicBlock *MBB : post_order(&MF)) {
>>> +    for (MachineBasicBlock::reverse_iterator MII = MBB->rbegin(),
>>> +                                             End = MBB->rend();
>>> +         MII != End;) {
>>> +      MachineInstr &MI = *MII++;
>>
>> I'm getting an assertion failure from the above loop.
>>
>> llc: ../include/llvm/ADT/ilist.h:244: llvm::ilist_iterator<NodeTy>&
>> llvm::ilist_iterator<NodeTy>::operator--() [with NodeTy =
>> llvm::MachineInstr]: Assertion `NodePtr && "--'d off the beginning of an
>> ilist!"' failed.
>>
>>
>> This is when I try to insert a new instruction and erase the old one,
>> in my select() implementation.  For example:
>>
>> BuildMI(*BB, &I, DL, TII.get(AMDGPU::FLAT_STORE_DWORD))
>>           .addOperand(I.getOperand(1))
>>           .addOperand(I.getOperand(0))
>>           .addImm(0)
>>           .addImm(0)
>>           .addImm(0);
>>
>> I.eraseFromParent();
>
> Ah, let me try that out in AArch64.  I also want to dump all inserted
> instructions; I'll do both.

Forgot to mention: for those following along at home, I took a stab at
fixing this in https://reviews.llvm.org/D22924.

-Ahmed

> -Ahmed
>
>> -Tom
>>
>>
>>
>>> +      DEBUG(dbgs() << "Selecting: " << MI << '\n');
>>> +      if (!ISel->select(MI))
>>> +        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.
>>> +    }
>>> +  }
>>> +