[EVM] Fix isCommutable flag.

llvm/lib/Target/EVM/EVMControlFlowGraph.h

@@ -169,12 +169,9 @@ struct CFG {
 
   struct BuiltinCall {
     MachineInstr *Builtin = nullptr;
-    // This contains commutable operand indexes, if any.
-    // Operand indexes correspond to a stack layout, i.e., the first use
-    // operand in the instruction definition is located on the stack top
-    // and has zero index.
-    std::optional<std::pair<unsigned, unsigned>> CommutableOpIndexes =
-        std::nullopt;
+    // True if this instruction has commutable operands. In EVM ISA
+    // commutable operands always take top two stack slots.
+    bool IsCommutable = false;
     bool TerminatesOrReverts = false;
   };
 

llvm/lib/Target/EVM/EVMControlFlowGraphBuilder.cpp

@@ -21,7 +21,6 @@
 #include "MCTargetDesc/EVMMCTargetDesc.h"
 #include "llvm/CodeGen/MachineFunction.h"
 
-#include <optional>
 #include <ostream>
 #include <variant>
 
@@ -211,17 +210,14 @@ void ControlFlowGraphBuilder::collectInstrOperands(const MachineInstr &MI,
   }
 }
 
-std::optional<std::pair<unsigned, unsigned>>
-ControlFlowGraphBuilder::getCommutableOpIndexes(const MachineInstr &MI) const {
+bool ControlFlowGraphBuilder::isCommutable(const MachineInstr &MI) const {
+  // In all the cases, commutable operands take top two stack slots.
+  // Even for ADDMOD and MULMOD instructions, because their MOD operand
+  // takes third slot counted from the top of stack.
   unsigned Opc = MI.getOpcode();
-  if (MI.isCommutable() || Opc == EVM::ADDMOD || Opc == EVM::MULMOD)
-    // Operand indexes correspond to the stack layout. In all the cases,
-    // commutable operands take two top stack slots. Even for ADDMOD and
-    // MULMOD instructions, because their MOD operand takes third slot from the
-    // TOS.
-    return std::pair<unsigned, unsigned>(0, 1);
-
-  return std::nullopt;
+  return (MI.isCommutable() || Opc == EVM::ADDMOD || Opc == EVM::MULMOD)
+             ? true
+             : false;
 }
 
 void ControlFlowGraphBuilder::handleMachineInstr(MachineInstr &MI) {
@@ -272,11 +268,9 @@ void ControlFlowGraphBuilder::handleMachineInstr(MachineInstr &MI) {
   default: {
     Stack Input, Output;
     collectInstrOperands(MI, &Input, &Output);
-    std::optional<std::pair<unsigned, unsigned>> CommutableOpIndexes =
-        getCommutableOpIndexes(MI);
     CurrentBlock->Operations.emplace_back(CFG::Operation{
         std::move(Input), std::move(Output),
-        CFG::BuiltinCall{&MI, CommutableOpIndexes, TerminatesOrReverts}});
+        CFG::BuiltinCall{&MI, isCommutable(MI), TerminatesOrReverts}});
   } break;
   }
 

llvm/lib/Target/EVM/EVMControlFlowGraphBuilder.h

@@ -42,8 +42,7 @@ class ControlFlowGraphBuilder {
   void handleReturn(const MachineInstr &MI);
   void handleBasicBlockSuccessors(MachineBasicBlock &MBB);
   StackSlot getDefiningSlot(const MachineInstr &MI, Register Reg) const;
-  std::optional<std::pair<unsigned, unsigned>>
-  getCommutableOpIndexes(const MachineInstr &MI) const;
+  bool isCommutable(const MachineInstr &MI) const;
   void collectInstrOperands(const MachineInstr &MI, Stack *Input,
                             Stack *Output) const;
 

llvm/lib/Target/EVM/EVMOptimizedCodeTransform.cpp

@@ -259,13 +259,14 @@ void EVMOptimizedCodeTransform::createOperationEntryLayout(
   // Check if we can choose cheaper stack shuffling if the Operation is an
   // instruction with commutable arguments.
   if (const auto *Inst = std::get_if<CFG::BuiltinCall>(&Op.Operation);
-      Inst && Inst->CommutableOpIndexes) {
+      Inst && Inst->IsCommutable) {
     // Get the stack layout before the instruction.
     const Stack &DefaultTargetStack = Layout.operationEntryLayout.at(&Op);
     size_t DefaultCost =
         EvaluateStackTransform(CurrentStack, DefaultTargetStack);
-    unsigned OpIdx1 = Inst->CommutableOpIndexes->first;
-    unsigned OpIdx2 = Inst->CommutableOpIndexes->second;
+
+    // Commutable operands always take top two stack slots.
+    const unsigned OpIdx1 = 0, OpIdx2 = 1;
     // Swap the commutable stack items and measure the stack shuffling cost
     // again.
     assert(DefaultTargetStack.size() > OpIdx1 &&