[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

This revision was automatically updated to reflect the committed changes. Closed by commit rG4a7aa252a32a: [X86][AsmParser] re-introduce 'offset' operator (authored by epastor). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71436/new/ https://review

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61144 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon question-circle color=gray} Unit tests: unknown. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings . {icon times-circle color=red} clang-forma

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 235620. epastor added a comment. - The three-argument version of find_if is std::find_if, and the qualification is required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71436/new/ https://reviews.llvm.org/D7

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61144 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 235619. epastor marked 5 inline comments as done. epastor added a comment. - Fix a few final issues. - Pack the AsmRewrite struct more optimally. - Only check if we can fuse rewrites with ones we haven't already reached! Repository: rG LLVM Github Mono

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor added inline comments. Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821 +getParser().parsePrimaryExpr(Val, End)) + return Error(Start, "unexpected token!"); + } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) { ---

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 235617. epastor added a comment. Rebase on top of HEAD - Add compatibility with dc5b614fa9a1 (which changed handling for call operands in x86 assembly) - Remove another artifact of local

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821 +getParser().parsePrimaryExpr(Val, End)) + return Error(Start, "unexpected token!"); + }

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed and 726 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon question-circle color=gray} Unit tests: unknown. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings . {icon times-circle color=red} clang-forma

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor added inline comments. Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821 +getParser().parsePrimaryExpr(Val, End)) + return Error(Start, "unexpected token!"); + } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) { ---

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 234567. epastor added a comment. - Removing accidental artifacts of local testing... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71436/new/ https://reviews.llvm.org/D71436 Files: clang/lib/Sema/SemaStmtAsm

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 234566. epastor marked 2 inline comments as done. epastor added a comment. Fix issues around enum values and labels - Only rewrite variables as offset operands; fixes crash due to conflicting rewrites for labels - Recognize inline assembly references to enum

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed and 726 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor added a comment. Thanks for feedback! Comment at: clang/test/CodeGen/ms-inline-asm-64.c:18 // CHECK: call void asm sideeffect inteldialect -// CHECK-SAME: mov [eax], $0 +// CHECK-SAME: mov qword ptr [eax], $0 // CHECK-SAME: "r,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 234371. epastor marked 6 inline comments as done. epastor added a comment. - Address refactoring comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71436/new/ https://reviews.llvm.org/D71436 Files: clang

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

rnk added a comment. Thanks! I made some comments. This code is convoluted. I don't think I can reason through all the edge cases, but this seems like an improvement. I'd suggest adding the recommended tests and any other corner case inputs you can think of, and after another round of review we

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed and 726 were skipped. {icon times-circle color=red} clang-format: fail. Please format your changes with clang-format by running `git-clang-format HEAD^` or apply this patch

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 234067. epastor added a comment. - Apply formatting fixes where reasonable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71436/new/ https://reviews.llvm.org/D71436 Files: clang/test/CodeGen/ms-inline-asm-64.

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed and 726 were skipped. {icon times-circle color=red} clang-format: fail. Please format your changes with clang-format by running `git-clang-format HEAD^` or apply this patch

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 233813. epastor added a comment. - Respect correct usage of offset. - Don't assume implicit sizing on offset. - Check that offset works in compound expressions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7143

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 60824 tests passed, 4 failed and 726 were skipped. failed: Clang.CodeGen/ms-inline-asm-64.c failed: Clang.CodeGen/ms-inline-asm.c failed: Clang.SemaTemplate/instantiation-depth-default.cpp failed: LLVM.Code

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 233806. epastor added a comment. - Fix Intel named operator parsing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71436/new/ https://reviews.llvm.org/D71436 Files: clang/test/CodeGen/ms-inline-asm.c clang/

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

epastor updated this revision to Diff 233786. epastor added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Fix tests expecting use of 'r' constraints for offset operands Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o