[PATCH] D126349: [TableGen] Remove code beads

[Sheng via Phabricator via llvm-commits]

0x59616e marked an inline comment as done.
0x59616e added inline comments.


================
Comment at: llvm/lib/Target/M68k/M68kInstrInfo.cpp:603
     const MachineOperand &MO) const {
-  assert(MO.isReg());
-  const auto *MI = MO.getParent();
-  const uint8_t *Beads = M68k::getMCInstrBeads(MI->getOpcode());
-  assert(*Beads);
-
-  // Only addressing mode k has (non-pc) register with PCRel
-  // So we're looking for EA Beads equal to
-  // `3Bits<011>_1Bit<1>_2Bits<11>`
-  // FIXME: There is an important caveat and two assumptions
-  // here: The caveat is that EA encoding always sit on the LSB.
-  // Where the assumptions are that if there are more than one
-  // operands, the EA encoding for the source operand always sit
-  // on the LSB. At the same time, k addressing mode can not be used
-  // on destination operand.
-  // The last assumption is kinda dirty so we need to find a way around
-  // it
-  const uint8_t EncEAk[3] = {0b011, 0b1, 0b11};
-  for (const uint8_t Pat : EncEAk) {
-    uint8_t Bead = *(Beads++);
-    if (!Bead)
-      return false;
-
-    switch (Bead & 0xF) {
-    default:
-      return false;
-    case M68kBeads::Bits1:
-    case M68kBeads::Bits2:
-    case M68kBeads::Bits3: {
-      uint8_t Val = (Bead & 0xF0) >> 4;
-      if (Val != Pat)
-        return false;
-    }
-    }
-  }
-  return true;
+  llvm_unreachable("unimplemented");
 }
----------------
myhsu wrote:
> We can't mark this function as unimplemented. I added this `isPCRelRegisterOperandLegal` hook when we were upstreaming the m68k backend. The story behind this was that, as the comments suggested, a logical operand that has addressing mode 'k' is consisting of 3 MachineOperand, for example:
> ```
> (87, %pc, %a1)
> ```
> And we mark all three MachineOperand as pc-rel due to the nature of addressing mode 'k'. But MachineVerifier was not happy about this b/c it was not expecting a register operand (i.e. `%a1` in the snippet above) to be marked as pc-rel. So this hook was added for MachineVerifier to check this very scenario. (The reason we didn't see this problem after the transition is because the new MC tests don't even get translated into MachineInstr).
> 
> One way to implement this function will be (manually) pattern matching against the opcode and the MachineOperand.
> 
> Here is a MachineInstr to test:
> ```
> name: test_move32jk
> body: |
>   bb.0:
>     MOV32jk $a1,  0, $a2, implicit-def $ccr
> ```
I use the operand number to distinguish between source and destination. And then use regular expression to check whether the instruction has 'k' addressing mode.


================
Comment at: llvm/utils/gn/secondary/llvm/utils/TableGen/BUILD.gn:15
     "CallingConvEmitter.cpp",
-    "CodeBeadsGen.cpp",
     "CodeEmitterGen.cpp",
----------------
myhsu wrote:
> Thanks for taking care GN as well!
I did some google, and realize that gn is a build system that can generate ninja build files.

Why do we have this, given that cmake can do the same thing ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126349/new/

https://reviews.llvm.org/D126349