ego added inline comments.
================ Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:135 + defm VANDN_V : VALU_IV_V_X_I<"vandn", 0b000001>; + def VBREV8_V : VALUVs2<0b010010, 0b01000, OPIVV, "vbrev8.v">; + defm VCLMUL_V : VALU_IV_V_X_VCLMUL<"vclmul", 0b001100>; ---------------- This ought to be OPMVV. https://github.com/riscv/riscv-crypto/blob/master/doc/vector/insns/vbrev8.adoc Doing OPIVV->OPMVV sets bit 13, adding 0x20 to the second byte in the CHECK-ENCODING tests for vbrev8 and vrev8 ``` vbrev8.v v10, v9, v0.t # CHECK-INST: vbrev8.v v10, v9, v0.t # CHECK-ENCODING: [0x57,0x25,0x94,0x48] # CHECK-ERROR: instruction requires the following: 'Zvkb' # CHECK-UNKNOWN: 57 25 94 48 <unknown> ... vrev8.v v10, v9, v0.t # CHECK-INST: vrev8.v v10, v9, v0.t # CHECK-ENCODING: [0x57,0xa5,0x94,0x48] # CHECK-ERROR: instruction requires the following: 'Zvkb' # CHECK-UNKNOWN: 57 a5 94 48 <unknown> ``` diff: ``` GIT_PAGER= git diff diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td index b44bf7476808..3477c8d860ac 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td +++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td @@ -132,10 +132,10 @@ def rnum_2_14 : Operand<XLenVT>, ImmLeaf<XLenVT, let Predicates = [HasStdExtZvkb] in { defm VANDN_V : VALU_IV_V_X_I<"vandn", 0b000001>; - def VBREV8_V : VALUVs2<0b010010, 0b01000, OPIVV, "vbrev8.v">; + def VBREV8_V : VALUVs2<0b010010, 0b01000, OPMVV, "vbrev8.v">; defm VCLMUL_V : VALU_IV_V_X_VCLMUL<"vclmul", 0b001100>; defm VCLMULH_V : VALU_IV_V_X_VCLMUL<"vclmulh", 0b001101>; - def VREV8_V : VALUVs2<0b010010, 0b01001, OPIVV, "vrev8.v">; + def VREV8_V : VALUVs2<0b010010, 0b01001, OPMVV, "vrev8.v">; defm VROL_V : VALU_IV_V_X<"vrol", 0b010101>; defm VROR_V : VALU_IV_V_X_I_VROR<"vror", 0b010100>; } // Predicates = [HasStdExtZvkb] diff --git a/llvm/test/MC/RISCV/rvv/rv64zvkb.s b/llvm/test/MC/RISCV/rvv/rv64zvkb.s index f4f7d9a038fc..d1600296e7e6 100644 --- a/llvm/test/MC/RISCV/rvv/rv64zvkb.s +++ b/llvm/test/MC/RISCV/rvv/rv64zvkb.s @@ -28,9 +28,9 @@ vandn.vi v10, v9, 7, v0.t vbrev8.v v10, v9, v0.t # CHECK-INST: vbrev8.v v10, v9, v0.t -# CHECK-ENCODING: [0x57,0x05,0x94,0x48] +# CHECK-ENCODING: [0x57,0x25,0x94,0x48] # CHECK-ERROR: instruction requires the following: 'Zvkb' -# CHECK-UNKNOWN: 57 05 94 48 <unknown> +# CHECK-UNKNOWN: 57 25 94 48 <unknown> vclmul.vv v10, v9, v8 # CHECK-INST: vclmul.vv v10, v9, v8 @@ -58,9 +58,9 @@ vclmulh.vx v10, v9, a0 vrev8.v v10, v9, v0.t # CHECK-INST: vrev8.v v10, v9, v0.t -# CHECK-ENCODING: [0x57,0x85,0x94,0x48] +# CHECK-ENCODING: [0x57,0xa5,0x94,0x48] # CHECK-ERROR: instruction requires the following: 'Zvkb' -# CHECK-UNKNOWN: 57 85 94 48 <unknown> +# CHECK-UNKNOWN: 57 a5 94 48 <unknown> vrol.vv v10, v9, v8, v0.t # CHECK-INST: vrol.vv v10, v9, v8, v0.t ``` ================ Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:138 + defm VCLMULH_V : VALU_IV_V_X_VCLMUL<"vclmulh", 0b001101>; + def VREV8_V : VALUVs2<0b010010, 0b01001, OPIVV, "vrev8.v">; + defm VROL_V : VALU_IV_V_X<"vrol", 0b010101>; ---------------- This ought to be OPMVV. https://github.com/riscv/riscv-crypto/blob/master/doc/vector/insns/vrev8.adoc ================ Comment at: llvm/test/MC/RISCV/rvv/rv64zvknh.s:19-20 +vsha2ms.vv v10, v9, v8 +# CHECK-INST-ZVKNHA: vsha2ms.vv v10, v9, v8 +# CHECK-INST-ZVKNHB: vsha2ms.vv v10, v9, v8 +# CHECK-ENCODING-ZVKNHA: [0x77,0x25,0x94,0xb6] ---------------- While I do understand why we want to run the tests with each of the two extensions declared, I don't understand the value of replicating the checks in the cases where there is no difference between A and B. I do not see any case where the expectations differ since the instructions exist in both cases, have the same encodings, and the error string covers both extensions. If we want to communicate that both behave the same, a comment would convey this more clearly. ================ Comment at: llvm/test/MC/RISCV/rvv/rv64zvkns.s:59 + +vaeskf1.vi v10, v9, 1 +# CHECK-INST: vaeskf1.vi v10, v9, 1 ---------------- craig.topper wrote: > ego wrote: > > If I replaces "v10" with "v0", the test fails with an assertion failure. My > > own patch uses a slightly different class hierarchy but hits the same > > assertion. > > > > > > ``` > > FAIL: LLVM :: MC/RISCV/rvv/rv64zvkns.s (3 of 8) > > ******************** TEST 'LLVM :: MC/RISCV/rvv/rv64zvkns.s' FAILED > > ******************** > > Script: > > -- > > : 'RUN: at line 1'; > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc > > -triple=riscv64 -show-encoding --mattr=+zve32x --mattr=+experimental-zvkns > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > | > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > --check-prefixes=CHECK-ENCODING,CHECK-INST > > : 'RUN: at line 3'; not > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc > > -triple=riscv64 -show-encoding > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > 2>&1 | > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > --check-prefix=CHECK-ERROR > > : 'RUN: at line 5'; > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc > > -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvkns > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > | > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-objdump > > -d --mattr=+zve32x --mattr=+experimental-zvkns - | > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > --check-prefix=CHECK-INST > > : 'RUN: at line 8'; > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc > > -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvkns > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > | > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-objdump > > -d - | > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > --check-prefix=CHECK-UNKNOWN > > -- > > Exit Code: 2 > > > > Command Output (stderr): > > -- > > Assertion failed: (isReg() && "This is not a register operand!"), function > > getReg, file MCInst.h, line 70. > > PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ > > and include the crash backtrace. > > Stack dump: > > 0. Program arguments: > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/llvm-mc > > -triple=riscv64 -show-encoding --mattr=+zve32x --mattr=+experimental-zvkns > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > Stack dump without symbol names (ensure you have llvm-symbolizer in your > > PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): > > 0 libLLVMSupport.dylib 0x00000001023015e8 > > llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 56 > > 1 libLLVMSupport.dylib 0x0000000102300840 > > llvm::sys::RunSignalHandlers() + 112 > > 2 libLLVMSupport.dylib 0x0000000102301c28 SignalHandler(int) + 304 > > 3 libsystem_platform.dylib 0x00000001914f82a4 _sigtramp + 56 > > 4 libsystem_pthread.dylib 0x00000001914c9cec pthread_kill + 288 > > 5 libsystem_c.dylib 0x00000001914032c8 abort + 180 > > 6 libsystem_c.dylib 0x0000000191402620 err + 0 > > 7 libLLVMRISCVAsmParser.dylib 0x0000000100666208 (anonymous > > namespace)::RISCVAsmParser::MatchAndEmitInstruction(llvm::SMLoc, unsigned > > int&, llvm::SmallVectorImpl<std::__1::unique_ptr<llvm::MCParsedAsmOperand, > > std::__1::default_delete<llvm::MCParsedAsmOperand>>>&, llvm::MCStreamer&, > > unsigned long long&, bool) (.cold.42) + 0 > > 8 libLLVMRISCVAsmParser.dylib 0x0000000100655084 (anonymous > > namespace)::RISCVAsmParser::MatchAndEmitInstruction(llvm::SMLoc, unsigned > > int&, llvm::SmallVectorImpl<std::__1::unique_ptr<llvm::MCParsedAsmOperand, > > std::__1::default_delete<llvm::MCParsedAsmOperand>>>&, llvm::MCStreamer&, > > unsigned long long&, bool) + 7268 > > 9 libLLVMMCParser.dylib 0x0000000100c36c08 (anonymous > > namespace)::AsmParser::parseAndMatchAndEmitTargetInstruction((anonymous > > namespace)::ParseStatementInfo&, llvm::StringRef, llvm::AsmToken, > > llvm::SMLoc) + 1356 > > 10 libLLVMMCParser.dylib 0x0000000100c2d1d0 (anonymous > > namespace)::AsmParser::parseStatement((anonymous > > namespace)::ParseStatementInfo&, llvm::MCAsmParserSemaCallback*) + 3492 > > 11 libLLVMMCParser.dylib 0x0000000100c27884 (anonymous > > namespace)::AsmParser::Run(bool, bool) + 500 > > 12 llvm-mc 0x000000010051ffc8 main + 6608 > > 13 dyld 0x000000019119fe50 start + 2544 > > FileCheck error: '<stdin>' is empty. > > FileCheck command line: > > /Users/ego/root-dirs/scratch/projects/llvm-project/build/bin/FileCheck > > /Users/ego/root-dirs/scratch/projects/llvm-project/llvm/test/MC/RISCV/rvv/rv64zvkns.s > > --check-prefixes=CHECK-ENCODING,CHECK-INST > > > > -- > > > > ``` > Looks like it ended up with `RVVConstraint = VMConstraint` due to the classes > it inherited. This caused validateInstruction to look for a mask operand that > doesn't exist. Need to force it to the NoConstraint. Thanks Craig, that "NoConstraint" worked for me. It would be good to check if the same issue exists with other instructions in their no-mask uses. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138807/new/ https://reviews.llvm.org/D138807 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
