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* %{{.*}})
----------------
rnk wrote:
> I guess this has the same semantics as it did before.
It does; this previously resolved to a "qword ptr" mov automatically, since $0
unpacked as a (forced) 64-bit register name. Now we need to specify the size of
the move, since the operand is an immediate rather than a register.
================
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)) {
----------------
rnk wrote:
> Please test this corner case, I imagine it looks like:
> mov eax, offset 3
Interesting. This corner case didn't trigger in that scenario; we get an
"expected identifier" error message with good source location, followed by
another error "use of undeclared label '3'" in debug builds... and in release
builds, we instead get a crash. On tracing the crash, it's a AsmStrRewrite
applying to a SMLoc not coming from the same string...
As near as I can tell, the issue is that we end up trying to parse "3" as a
not-yet-declared label, as such expand it to `__MSASMLABEL_.${:uid}__3`, and
then end up in a bad state because the operand rewrite is applying to the
expanded symbol... which isn't in the same AsmString. If you use an actual
undeclared label, you hit the same crash in release builds.
This is going to take some work; I'll get back to it in a day or two.
================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1825
+ } else if (Info.isKind(InlineAsmIdentifierInfo::IK_EnumVal)) {
+ return Error(Start, "offset operator cannot yet handle constants");
+ }
----------------
rnk wrote:
> Please add a test for this corner case, I'm curious to see if the error
> reporting really works.
This error reporting doesn't work, in fact. We instead get "cannot take the
address of an rvalue of type 'int'", with bad source location. Will investigate
why we end up there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71436/new/
https://reviews.llvm.org/D71436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
