[GlobalISel][TableGen] Add BitsPerByte value

[ehjogab]

Summary

In some targets, the smallest addressable unit does not consist of 8
bits. For such targets, the matcher code for G_SEXTLOAD and G_ZEXTLOAD
is incorrect and never yield a match. This patch addresses this problem
by introducing a new value BitsPerByte, thereby allowing the 8-bit
assumption to be removed from TableGen.

Change-Id: Ic6a3e6976a960c8ef65b4a7dbb065b2529d07dd9

[Paul-C-Anagnostopoulos]

llvm/utils/TableGen/GlobalISelEmitter.cpp

3816

Could you fix the English in the comment above?

I should know, but which getSizeInBits() is this calling? There are two definitions, both with the 8 hardwired. One is in CodeGen/MachineMemOperand.h and the other in ir/DataLayout.h.

Ah, is it calling the one in LowLevelTypeImpl.h? That one doesn't have a hardwired 8, although the function getSizeInBytes does.

[ehjogab]

Fix lint warnings
Add testcase

[ehjogab]

I believe it is calling the one in LowLevelTypeImpl.h.

I agree that things become confusing when using hardcoded 8s in some places and then not in other places. Preferably, all hardcoded 8s should be replaced by something akin to this; we have done this in the codebase for our target but been unable to upstream the patch (for reasons unknown to me).

[lebedev.ri]

This should be an RFC to llvm-dev, because this tries to silently lift the restriction that byte is 8 bits,
which is prevalent through llvm. And i do mean silently, because this has been brought up before a number of times.

llvm/include/llvm/Target/Target.td

class Target {
   // AssemblyWriters - The AsmWriter instances available for this target.
   list<AsmWriter> AssemblyWriters = [DefaultAsmWriter];
 
   // AllowRegisterRenaming - Controls whether this target allows
   // post-register-allocation renaming of registers.  This is done by
   // setting hasExtraDefRegAllocReq and hasExtraSrcRegAllocReq to 1
   // for all opcodes if this flag is set to 0.
   int AllowRegisterRenaming = 0;
+
+  // BitsPerByte - Specifies the size (in bits) of the smallest addressable
+  // unit in the target. Typically this is 8 bits, which is the default value.
+  int BitsPerByte = 8;
 }
 
 //===----------------------------------------------------------------------===//
 // SubtargetFeature - A characteristic of the chip set.
 //
 class SubtargetFeature<string n, string a,  string v, string d,
                        list<SubtargetFeature> i = []> {
   // Name - Feature name.  Used by command line (-mattr=) to determine the

llvm/utils/TableGen/CodeGenTarget.h

public:
   ///
   Record *getInstructionSet() const;
 
   /// getAllowRegisterRenaming - Return the AllowRegisterRenaming flag value for
   /// this target.
   ///
   bool getAllowRegisterRenaming() const;
 
+  /// getBitsPerByte - Return the BitsPerByte int value for this target.
+  ///
+  int getBitsPerByte() const;
+
   /// getAsmParser - Return the AssemblyParser definition for this target.
   ///
   Record *getAsmParser() const;
 
   /// getAsmParserVariant - Return the AssemblyParserVariant definition for
   /// this target.
   ///
   Record *getAsmParserVariant(unsigned i) const;

llvm/utils/TableGen/CodeGenTarget.cpp

 Record *CodeGenTarget::getInstructionSet() const {
   return TargetRec->getValueAsDef("InstructionSet");
 }
 
 bool CodeGenTarget::getAllowRegisterRenaming() const {
   return TargetRec->getValueAsInt("AllowRegisterRenaming");
 }
 
+int CodeGenTarget::getBitsPerByte() const {
+  return TargetRec->getValueAsInt("BitsPerByte");
+}
+
 /// getAsmParser - Return the AssemblyParser definition for this target.
 ///
 Record *CodeGenTarget::getAsmParser() const {
   std::vector<Record*> LI = TargetRec->getValueAsListOfDefs("AssemblyParsers");
   if (AsmParserNum >= LI.size())
     PrintFatalError("Target does not have an AsmParser #" +
                     Twine(AsmParserNum) + "!");
   return LI[AsmParserNum];

llvm/utils/TableGen/GlobalISelEmitter.cpp

 //===- GlobalISelEmitter.cpp - Generate an instruction selector -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file
if (Predicate.getMemoryVT() != nullptr) {
       Optional<LLTCodeGen> MemTyOrNone =
           MVTToLLT(getValueType(Predicate.getMemoryVT()));
 
       if (!MemTyOrNone)
         return failedImport("MemVT could not be converted to LLT");
 
       // MMO's work in bytes so we must take care of unusual types like i1
       // don't round down.
       unsigned MemSizeInBits =
-          llvm::alignTo(MemTyOrNone->get().getSizeInBits(), 8);
+          llvm::alignTo(MemTyOrNone->get().getSizeInBits(),
+                        Target.getBitsPerByte());
 
-      InsnMatcher.addPredicate<MemorySizePredicateMatcher>(0,
-                                                           MemSizeInBits / 8);
+      InsnMatcher
+          .addPredicate<MemorySizePredicateMatcher>(0,
+                                                    MemSizeInBits /
+                                                    Target.getBitsPerByte());
       return InsnMatcher;
     }
   }
 
   if (Predicate.isLoad() || Predicate.isStore()) {
     // No check required. A G_LOAD/G_STORE is an unindexed load.
     if (Predicate.isUnindexed())
       return InsnMatcher;

 
 //===----------------------------------------------------------------------===//
 
 namespace llvm {
 void EmitGlobalISel(RecordKeeper &RK, raw_ostream &OS) {
   GlobalISelEmitter(RK).run(OS);
 }
 } // End llvm namespace
+