[PATCH] D42132: [RISCV] Fixed setting predicates for compressed instructions.

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

Thanks for this - definitely a real problem that we want to fix.

Looking around, I see Mips, AMDGPU and Nios define a 'PredicateControl' class which they use to get more fine-grained control over predicates. I think this is more complex than what we require, and your fix of just explicitly overriding Predicates with a complete list is better for our use-case.

So I'm happy with the .td changes. For the test changes, could we extend the existing tests instead? e.g. modify test/MC/RISCV/rv32fc-valid.s to add new RUN lines for mattr with only +c, and new check lines for the error? So rv32fc-valid.s would look like:

  # RUN: llvm-mc %s -triple=riscv32 -mattr=+c,+f -riscv-no-aliases -show-encoding \
  # RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
  # RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+c,+f < %s \
  # RUN:     | llvm-objdump -mattr=+c,+f -riscv-no-aliases -d - \
  # RUN:     | FileCheck -check-prefix=CHECK-INST %s
  # RUN: not llvm-mc -triple riscv32 -mattr=+c < %s 2>&1 \
  # RUN:     | FileCheck -check-prefix=CHECK-NOF %s
  
  
  # CHECK-INST: c.flwsp  fs0, 252(sp)
  # CHECK: encoding: [0x7e,0x74]
  # CHECK-NOF: :[[@LINE+1]]:1: error: instruction use requires an option to be enabled
  c.flwsp  fs0, 252(sp)
  # CHECK-INST: c.fswsp  fa7, 252(sp)
  # CHECK: encoding: [0xc6,0xff]
  # CHECK-NOF: :[[@LINE+1]]:1: error: instruction use requires an option to be enabled
  c.fswsp  fa7, 252(sp)
  
  # CHECK-INST: c.flw  fa3, 124(a5)
  # CHECK: encoding: [0xf4,0x7f]
  # CHECK-NOF: :[[@LINE+1]]:1: error: instruction use requires an option to be enabled
  c.flw  fa3, 124(a5)
  # CHECK-INST: c.fsw  fa2, 124(a1)
  # CHECK: encoding: [0xf0,0xfd]
  # CHECK-NOF: :[[@LINE+1]]:1: error: instruction use requires an option to be enabled
  c.fsw  fa2, 124(a1)


https://reviews.llvm.org/D42132