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

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:790
+                                          "EnableSeqCstTrailingFence",
+                                          "true",
+                                          "Enable trailing fence for seq-cst store.">;
----------------
craig.topper wrote:
> 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.
Ah, thanks. I knew I was missing something in how this was intended to work. I've looked a few times through the tablegen documentation and tried to walk through the relevant definitions in the tablegen files & C++, but I've been unable to figure out exactly where I should have been looking. Could you point me to the relevant bits? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149486