[PATCH] D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw
JF Bastien via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 17 11:11:40 PST 2020
jfb added a comment.
In D72741#1826996 <https://reviews.llvm.org/D72741#1826996>, @flaub wrote:
> I don't think this commit should be about making changes to LLVM IR. I can do that in a followup, but this commit should be in sync with whatever LLVM IR is today. There seems to be consensus with this approach.
Agreed this shouldn't make changes to LLVM IR. However, I'd like to understand whether:
1. a change would occur here first (in this patch, to MLIR), and then in LLVM IR (and who would lead this)
2. in MLIR and LLVM IR at the same time, as a follow-up, and who would lead this
3. never
You're making a new IR, and that's exactly the right time to revisit basic things like this instead of copying LLVM IR. This is so minor, I'm baffled by the pushback. You don't have consensus as evidenced by this discussion, and unwillingness to engage in such a basic question is super worrying. Please don't copy LLVM IR and instead understand why it's designed in a certain manner and what mistakes (however minor) it might contain. I'm not blocking this patch indefinitely, I'm asking you to answer these questions before proceeding. That should take a few minutes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72741/new/
https://reviews.llvm.org/D72741
More information about the llvm-commits
mailing list