xiangzhangllvm added inline comments.
================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1759
// It is widely common for MS InlineAsm to use a global variable and one/two
// registers in a mmory expression, and though unaccessible via rip/eip.
if (IsGlobalLV && (BaseReg || IndexReg)) {
----------------
skan wrote:
> xiangzhangllvm wrote:
> > Let me generally tell out my understand here, (If wrong PLS correct me)
> > Here from the comments we can see, the old code want to keep the origin
> > symbol of global variable to let linker (relocation) handle it. Here you
> > describe it with a pointer (with decl), it change to form of $ID <-->
> > (decl), So which need constrain it with "*m". But if the pointer can not be
> > access from BaseReg(Rip) + Index(Ip) how do you descript the pointer you
> > generate out ?
> >
> I think you may misunderstand this code.
>
> This code handles the memory that can not be represented by Disp(RIP) b/c
> there is already a BaseReg or IndexReg there.
>
> Before this patch, the memory is represented like `arr[edx*4]` and there is
> no identifer bound to it. And after this patch, we bind the memory to
> identifer arr.
Yes, that is why I mean t change ( arr[edx*4] ) to form of $ID <--> (decl). So
what the problem if we let the old form ( arr[edx*4] ) being ?
================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2554-2555
unsigned IndexReg = SM.getIndexReg();
+ if (IndexReg && BaseReg == X86::RIP)
+ BaseReg = 0;
unsigned Scale = SM.getScale();
----------------
skan wrote:
> xiangzhangllvm wrote:
> > The change here looks too arbitrary. For global address it is ok to drop
> > the base, it mainly fetch from offset. but if here not global variable?
> RIP and IndexReg can never be used together according to design of X86
> instruction. The BaseReg is set to RIP b/c we add the constaint "*m" in the
> MS-inline assembly. So we can drop it safely.
>
> I acknowledge it's not the best way to do it, but it's simplest. Similary,
> you can see the line 2560-2562. RSP can never be a IndexReg, but we just
> swap the BaseReg w/ IndexReg b/c it is not handled well in previous phases.
>
> RIP and IndexReg can never be used together according to design of X86
> instruction.
Good answer!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113096/new/
https://reviews.llvm.org/D113096
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
