Apply feedback

JanekvO · JanekvO · commit 7eaae11fe880 · 2024-03-06T16:37:07.000Z
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -8287,16 +8287,16 @@ void AMDGPUAsmParser::onBeginOfFile() {
 bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
   using AGVK = AMDGPUVariadicMCExpr::AMDGPUVariadicKind;
 
-  auto parseVariadicExpr = [&](AGVK Kind, const MCExpr *&res, SMLoc &EndLoc) {
-    std::vector<const MCExpr *> Exprs;
+  auto ParseVariadicExpr = [&](AGVK Kind, const MCExpr *&Result,
+                               SMLoc &EndLoc) {
+    SmallVector<const MCExpr *, 4> Exprs;
     while (true) {
-      if (isToken(AsmToken::RParen)) {
+      if (trySkipToken(AsmToken::RParen)) {
         if (Exprs.empty()) {
           Error(getToken().getLoc(), "empty max/or expression");
           return true;
         }
-        lex();
-        res = AMDGPUVariadicMCExpr::create(Kind, Exprs, getContext());
+        Result = AMDGPUVariadicMCExpr::create(Kind, Exprs, getContext());
         return false;
       }
       const MCExpr *Expr;
@@ -8320,7 +8320,7 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
     if (VK != AGVK::AGVK_None && peekToken().is(AsmToken::LParen)) {
       lex(); // Eat 'max'/'or'
       lex(); // Eat '('
-      return parseVariadicExpr(VK, Res, EndLoc);
+      return ParseVariadicExpr(VK, Res, EndLoc);
     }
   }
   return getParser().parsePrimaryExpr(Res, EndLoc, nullptr);
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -18,8 +18,7 @@ using namespace llvm;
 
 const AMDGPUVariadicMCExpr *
 AMDGPUVariadicMCExpr::create(AMDGPUVariadicKind Kind,
-                             const std::vector<const MCExpr *> &Args,
-                             MCContext &Ctx) {
+                             ArrayRef<const MCExpr *> Args, MCContext &Ctx) {
   return new (Ctx) AMDGPUVariadicMCExpr(Kind, Args);
 }
 
@@ -41,46 +40,47 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
     OS << "max(";
     break;
   }
-  for (auto it = Args.begin(); it != Args.end(); ++it) {
-    (*it)->print(OS, MAI, /*InParens=*/false);
-    if ((it + 1) != Args.end())
+  for (auto It = Args.begin(); It != Args.end(); ++It) {
+    (*It)->print(OS, MAI, /*InParens=*/false);
+    if ((It + 1) != Args.end())
       OS << ", ";
   }
   OS << ")";
 }
 
 bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
     MCValue &Res, const MCAsmLayout *Layout, const MCFixup *Fixup) const {
-  int64_t total = 0;
+  int64_t Total = INT64_MIN;
 
   auto Op = [this](int64_t Arg1, int64_t Arg2) -> int64_t {
     switch (Kind) {
     default:
       llvm_unreachable("Unknown AMDGPUVariadicMCExpr kind.");
     case AGVK_Max:
-      return Arg1 > Arg2 ? Arg1 : Arg2;
+      return std::max(Arg1, Arg2);
     case AGVK_Or:
-      return (!!Arg1) || (!!Arg2);
+      return Arg1 || Arg2;
     }
   };
 
   for (const MCExpr *Arg : Args) {
     MCValue ArgRes;
-    if (!Arg->evaluateAsRelocatable(ArgRes, Layout, Fixup))
+    if (!Arg->evaluateAsRelocatable(ArgRes, Layout, Fixup) ||
+        !ArgRes.isAbsolute())
       return false;
-    if (!ArgRes.isAbsolute())
-      return false;
-    total = Op(total, ArgRes.getConstant());
+
+    if (Total == INT64_MIN)
+      Total = ArgRes.getConstant();
+    Total = Op(Total, ArgRes.getConstant());
   }
 
-  Res = MCValue::get(total);
+  Res = MCValue::get(Total);
   return true;
 }
 
 void AMDGPUVariadicMCExpr::visitUsedExpr(MCStreamer &Streamer) const {
-  for (const MCExpr *Arg : Args) {
+  for (const MCExpr *Arg : Args)
     Streamer.visitUsedExpr(*Arg);
-  }
 }
 
 MCFragment *AMDGPUVariadicMCExpr::findAssociatedFragment() const {
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -9,43 +9,44 @@
 #ifndef LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
 #define LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
 
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/MC/MCExpr.h"
 
 namespace llvm {
 
-class AMDGPUMCExpr : public MCTargetExpr {
-  static bool classof(const MCExpr *E) {
-    return E->getKind() == MCExpr::Target;
-  }
-  void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
-};
-
-class AMDGPUVariadicMCExpr : public AMDGPUMCExpr {
+/// AMDGPU target specific variadic MCExpr operations.
+///
+/// Takes in a minimum of 1 argument to be used with an operation. The supported
+/// operations are:
+///   - (logic) or
+///   - max
+///
+class AMDGPUVariadicMCExpr : public MCTargetExpr {
 public:
   enum AMDGPUVariadicKind { AGVK_None, AGVK_Or, AGVK_Max };
 
 private:
   AMDGPUVariadicKind Kind;
-  std::vector<const MCExpr *> Args;
+  SmallVector<const MCExpr *, 2> Args;
 
-  AMDGPUVariadicMCExpr(AMDGPUVariadicKind Kind,
-                       const std::vector<const MCExpr *> &Args)
+  AMDGPUVariadicMCExpr(AMDGPUVariadicKind Kind, ArrayRef<const MCExpr *> Args)
       : Kind(Kind), Args(Args) {
-    assert(Args.size() >= 1 && "Can't take the maximum of 0 expressions.");
+    assert(Args.size() >= 1 && "Needs a minimum of one expression.");
   }
 
 public:
-  static const AMDGPUVariadicMCExpr *
-  create(AMDGPUVariadicKind Kind, const std::vector<const MCExpr *> &Args,
-         MCContext &Ctx);
+  static const AMDGPUVariadicMCExpr *create(AMDGPUVariadicKind Kind,
+                                            ArrayRef<const MCExpr *> Args,
+                                            MCContext &Ctx);
 
-  static const AMDGPUVariadicMCExpr *
-  createOr(const std::vector<const MCExpr *> &Args, MCContext &Ctx) {
+  static const AMDGPUVariadicMCExpr *createOr(ArrayRef<const MCExpr *> Args,
+                                              MCContext &Ctx) {
     return create(AMDGPUVariadicKind::AGVK_Or, Args, Ctx);
   }
 
-  static const AMDGPUVariadicMCExpr *
-  createMax(const std::vector<const MCExpr *> &Args, MCContext &Ctx) {
+  static const AMDGPUVariadicMCExpr *createMax(ArrayRef<const MCExpr *> Args,
+                                               MCContext &Ctx) {
     return create(AMDGPUVariadicKind::AGVK_Max, Args, Ctx);
   }
 
@@ -57,8 +58,12 @@ class AMDGPUVariadicMCExpr : public AMDGPUMCExpr {
                                  const MCFixup *Fixup) const override;
   void visitUsedExpr(MCStreamer &Streamer) const override;
   MCFragment *findAssociatedFragment() const override;
+  static bool classof(const MCExpr *E) {
+    return E->getKind() == MCExpr::Target;
+  }
+  void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
 };
 
 } // end namespace llvm
 
-#endif // LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
+#endif // LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd.s b/llvm/test/MC/AMDGPU/mcexpr_amd.s
@@ -15,6 +15,9 @@
 // OBJDUMP-NEXT: 000000000000000a l       *ABS*  0000000000000000 max_literals
 // OBJDUMP-NEXT: 000000000000000f l       *ABS*  0000000000000000 max_with_max_sym
 // OBJDUMP-NEXT: 000000000000000f l       *ABS*  0000000000000000 max
+// OBJDUMP-NEXT: ffffffffffffffff l       *ABS*  0000000000000000 neg_one
+// OBJDUMP-NEXT: ffffffffffffffff l       *ABS*  0000000000000000 max_neg_numbers
+// OBJDUMP-NEXT: ffffffffffffffff l       *ABS*  0000000000000000 max_neg_number
 // OBJDUMP-NEXT: 0000000000000003 l       *ABS*  0000000000000000 max_with_subexpr
 // OBJDUMP-NEXT: 0000000000000006 l       *ABS*  0000000000000000 max_as_subexpr
 // OBJDUMP-NEXT: 0000000000000005 l       *ABS*  0000000000000000 max_recursive_subexpr
@@ -51,6 +54,13 @@
 .set max_literals, max(1,2,3,4,5,6,7,8,9,10)
 .set max_with_max_sym, max(max, 4, 3, one, two)
 
+// ASM: .set max_neg_numbers, -1
+// ASM: .set max_neg_number, -1
+
+.set neg_one, -1
+.set max_neg_numbers, max(-5, -4, -3, -2, neg_one)
+.set max_neg_number, max(neg_one)
+
 // ASM: .set max_with_subexpr, 3
 // ASM: .set max_as_subexpr, 1+(max(4, 3, five))
 // ASM: .set max_recursive_subexpr, max(max(1, four), 3, max_expression_all)
