Add missing cases in RISCVMCExpr::getVariantKindName

[craig.topper]

Is it possible to test this?

[jrtc27]

I think these are just for debug output? Though maybe they appear in MIR too?

[sepavloff]

Updated patch

• changed switch statement so that adding new item to VariantKind would cause compilation warning if the respective case is not added.
• updated RISCVMCExpr::getVariantKindForName as well.

[craig.topper]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp

176

Can this just be llvm_unreachable and then you wouldn't need the first assert?

[sepavloff]

In D98929#2637766, @craig.topper wrote:

Is it possible to test this?

I do not know how to test this. But I changed the patch so that if a new item would be added to VariantKind and that item would not be handled in the switch statement, compilation would give a warning.

In D98929#2637769, @jrtc27 wrote:

I think these are just for debug output? Though maybe they appear in MIR too?

I got crash when executed llc -debug -filetype=obj. I am not sure if it can be found in MIR, but this looks unlikely, as it is an MC layer feature.

[sepavloff]

Use llvm_unreachable instead of bogus return

[sepavloff]

Removed unneeded assert

[sepavloff]

Does this tiny patch require some additional changes?

[craig.topper]

LGTM

[craig.topper]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp

136

Looking again. The only caller of this is RISCVAsmParser::parseOperandWithModifier. Are we allowing something additional in assembly parsing with this change? Is that something we should test?

[sepavloff]

Remove changes in RISCVMCExpr::getVariantKindForName

[craig.topper]

LGTM. Thanks!

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h

enum VariantKind {
     VK_RISCV_TPREL_LO,
     VK_RISCV_TPREL_HI,
     VK_RISCV_TPREL_ADD,
     VK_RISCV_TLS_GOT_HI,
     VK_RISCV_TLS_GD_HI,
     VK_RISCV_CALL,
     VK_RISCV_CALL_PLT,
     VK_RISCV_32_PCREL,
-    VK_RISCV_Invalid
+    VK_RISCV_Invalid // Must be the last item
   };
 
 private:
   const MCExpr *Expr;
   const VariantKind Kind;
 
   int64_t evaluateAsInt64(int64_t Value) const;
 

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp

return StringSwitch<RISCVMCExpr::VariantKind>(name)
       .Case("pcrel_lo", VK_RISCV_PCREL_LO)
       .Case("pcrel_hi", VK_RISCV_PCREL_HI)
       .Case("got_pcrel_hi", VK_RISCV_GOT_HI)
       .Case("tprel_lo", VK_RISCV_TPREL_LO)
       .Case("tprel_hi", VK_RISCV_TPREL_HI)
       .Case("tprel_add", VK_RISCV_TPREL_ADD)
       .Case("tls_ie_pcrel_hi", VK_RISCV_TLS_GOT_HI)
       .Case("tls_gd_pcrel_hi", VK_RISCV_TLS_GD_HI)
+      .Case("call", VK_RISCV_CALL)
+      .Case("call_plt", VK_RISCV_CALL_PLT)
+      .Case("32_pcrel", VK_RISCV_32_PCREL)
       .Default(VK_RISCV_Invalid);
 }
 
 StringRef RISCVMCExpr::getVariantKindName(VariantKind Kind) {
+  // When adding new item to VariantKind, getVariantKindForName must also be
+  // updated.
   switch (Kind) {
-  default:
+  case VK_RISCV_Invalid:
+  case VK_RISCV_None:
     llvm_unreachable("Invalid ELF symbol kind");
   case VK_RISCV_LO:
     return "lo";
   case VK_RISCV_HI:
     return "hi";
   case VK_RISCV_PCREL_LO:
     return "pcrel_lo";
   case VK_RISCV_PCREL_HI:
     return "pcrel_hi";
   case VK_RISCV_GOT_HI:
     return "got_pcrel_hi";
   case VK_RISCV_TPREL_LO:
     return "tprel_lo";
   case VK_RISCV_TPREL_HI:
     return "tprel_hi";
   case VK_RISCV_TPREL_ADD:
     return "tprel_add";
   case VK_RISCV_TLS_GOT_HI:
     return "tls_ie_pcrel_hi";
   case VK_RISCV_TLS_GD_HI:
     return "tls_gd_pcrel_hi";
+  case VK_RISCV_CALL:
+    return "call";
+  case VK_RISCV_CALL_PLT:
+    return "call_plt";
+  case VK_RISCV_32_PCREL:
+    return "32_pcrel";
   }
+  llvm_unreachable("Invalid ELF symbol kind");
 }
 
 static void fixELFSymbolsInTLSFixupsImpl(const MCExpr *Expr, MCAssembler &Asm) {
   switch (Expr->getKind()) {
   case MCExpr::Target:
     llvm_unreachable("Can't handle nested target expression");
     break;
   case MCExpr::Constant: