[PATCH] D157416: [RISCV][GlobalISel] Legalize multiplication

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp:112
+
+    getActionDefinitionsBuilder({G_SMULH, G_UMULH}).legalFor({XLenLLT}).lower();
+  } else {
----------------
Why did we stray from the usual formatting here?


================
Comment at: llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-mul-ext.mir:5
+# RUN: llc -mattr=+zmmul -mtriple=riscv32 -run-pass=legalizer %s -o - \
+# RUN: | FileCheck %s -check-prefix=CHECK-ZMMUL
+---
----------------
Can +m and +zmmul share the same check prefix?


================
Comment at: llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-mul-ext.mir:10
+  bb.0.entry:
+    ; CHECK-I-LABEL: name: mul_i7
+    ; CHECK-I: [[COPY:%[0-9]+]]:_(s32) = COPY $x10
----------------
CHECK-I is not a valid prefix for this test.


================
Comment at: llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-mul-ext.mir:3
+# RUN: llc -mattr=+m -mtriple=riscv64 -run-pass=legalizer %s -o - \
+# RUN: | FileCheck %s -check-prefix=CHECK-IM
+# RUN: llc -mattr=+zmmul -mtriple=riscv64 -run-pass=legalizer %s -o - \
----------------
Check prefixes in this test have an `I` in them that the rv32 tests don't have. We should be consistent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157416