[PATCH] D116156: [AArch64] Adding "armv8.8-a" BC instruction.

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Can you please site the relevant section of the Arm ARM for these extensions in the commit message?

My copy ver `ARM DDI 0487E.a (ID070919)` doesn't seem to mention these. (Checked § C4.1.3 and § A2).

My copy version `ARM DDI 0487G.b (ID072021)` doesn't seem to mention 8.8, so I can't personally verify the encodings. But that's fine if another reviewer perhaps can?



================
Comment at: llvm/lib/Target/AArch64/AArch64.td:420
+def FeatureHBC : SubtargetFeature<"hbc", "HasHBC",
+    "true", "Enable Armv8.8-A hinted conditional branch instruction">;
+
----------------
Should this be `hinted branch conditional` to match HBC, or does the ARM ARM refer to it as such?


================
Comment at: llvm/test/MC/AArch64/armv8.8a-hbc.s:3-4
+// RUN: llvm-mc -triple aarch64-none-linux-gnu -show-encoding -mattr=+v8.8a < %s | FileCheck %s
+// RUN: not llvm-mc -triple aarch64-none-linux-gnu < %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO-HBC-ERR %s < %t
+
----------------
Why do you need a temporary file (`%t`) here? Can't you just use a pipe as above?


================
Comment at: llvm/test/MC/AArch64/armv8.8a-hbc.s:22
+        bc.le lbl
+        bc.al lbl
+
----------------
Do you need to define `lbl` somewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116156