[PATCH] D149486: [RISCV] Strengthen atomic ordering for sequentially consistent stores

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:790
+                                          "EnableSeqCstTrailingFence",
+                                          "true",
+                                          "Enable trailing fence for seq-cst store.">;
----------------
paulkirth wrote:
> paulkirth wrote:
> > I think I set the default wrong here
> so .. the behavior I'm observing here seems wrong. Passing `"true"` here actually seems to cause the generated macro to be `GET_SUBTARGETINFO_MACRO(EnableSeqCstTrailingFence, false, enableSeqCstTrailingFence)`, which is what we want, but I would have thought that would only happen if we passed `"false"` here ... am I missing something in how the tablegen is working w/ `SubtargetFeature`?
The value in tablegen is not the default value. It’s the value to represent enabled. I think it can be “true” or an integer/enum. “true” means it’s a bool that starts as false. Anything else is an integer that’s starts at 0.

For the integer/enum case you can have multiple features touching the same field that all get maxed together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149486