[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

jyknight requested changes to this revision. jyknight added a comment. This revision now requires changes to proceed. I believe this is abandoned, correct? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145416/new/ https://reviews.llvm.org/D145416

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

jyknight added a comment. Also, I note the doc says it's useful for `for “load address” and “push address” instructions` (note, "load address" means e.g. x86 "lea" instruction) -- which should NOT be dependent upon the value stored in the memory. The x86 backend actually uses a "Ts" constraint

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers added a comment. https://lore.kernel.org/lkml/alpine.LFD.2.01.0908011214330.3304@localhost.localdomain/ has historical context around the introduction of "p" in the kernel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145416/new/

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers added a comment. In D145416#4173385 , @nickdesaulniers wrote: > In D145416#4173368 , > @nickdesaulniers wrote: > >> Looking at why the kernel ever used it, it looks like: >>

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers added a comment. In D145416#4173368 , @nickdesaulniers wrote: > Looking at why the kernel ever used it, it looks like: >

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers added a comment. In D145416#4173258 , @jyknight wrote: >> ‘p’ in the constraint must be accompanied by address_operand as the >> predicate in the match_operand. This predicate interprets the mode specified >> in the match_operand as the

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

jyknight added a comment. > ‘p’ in the constraint must be accompanied by address_operand as the predicate > in the match_operand. This predicate interprets the mode specified in the > match_operand as the mode of the memory reference for which the address would > be valid. How do you do that

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers added a comment. In D145416#4173250 , @jyknight wrote: > It looks to me from GCC that constraint 'p' is intended to be used > internally, not for inline-asm. I question whether the kernel should be using > it, and whether we should even

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

jyknight added a comment. It looks to me from GCC that constraint 'p' is intended to be used internally, not for inline-asm. I question whether the kernel should be using it, and whether we should even implement it at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers updated this revision to Diff 502820. nickdesaulniers added a comment. - add test additional case for Sema Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145416/new/ https://reviews.llvm.org/D145416 Files:

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers planned changes to this revision. nickdesaulniers added inline comments. Comment at: clang/test/Sema/inline-asm-validate.c:9 + // inputs. + asm (""::"p"(t), "p"(p)); } I should add a test case for `"p"()`. Repository: rG LLVM Github

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers added inline comments. Comment at: clang/lib/Basic/TargetInfo.cpp:747-748 break; case 'g': // general register, memory operand or immediate integer. case 'X': // any operand. Info.setAllowsRegister(); nickdesaulniers

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers added inline comments. Comment at: clang/lib/Basic/TargetInfo.cpp:747-748 break; case 'g': // general register, memory operand or immediate integer. case 'X': // any operand. Info.setAllowsRegister(); I wonder if we should

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers updated this revision to Diff 502781. nickdesaulniers added a comment. - add sema test to validate 'p' inputs as per @efriedma Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145416/new/ https://reviews.llvm.org/D145416 Files:

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers planned changes to this revision. nickdesaulniers added a comment. In D145416#4172799 , @efriedma wrote: > I think this is going to change what inputs Sema will accept for "p". If > that's intentional, please add test coverage.

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

efriedma added a comment. I think this is going to change what inputs Sema will accept for "p". If that's intentional, please add test coverage. Otherwise, please make a narrower change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145416/new/

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

nickdesaulniers created this revision. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC doesn't CSE inline asm when 'p' is used on inputs, neither should clang. In order to do so, we must not