<div dir="auto">Yes, right! There's x64 fastcall for everything. Ok, I'll remove it later, thanks!</div><br><div class="gmail_quote"><div dir="ltr">Am So., 30. Dez. 2018, 09:26 hat Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> geschrieben:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>Sorry, this comment was supposed to be deleted after I realized i was wrong.</div><div><br><div class="gmail_quote"><div dir="ltr">On Sat, Dec 29, 2018 at 9:12 PM Aleksandr Urakov via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank" rel="noreferrer">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">aleksandr.urakov marked an inline comment as done.<br>
aleksandr.urakov added inline comments.<br>
<br>
<br>
================<br>
Comment at: lit/SymbolFile/NativePDB/ast-methods.cpp:28<br>
+// CHECK: |-CXXRecordDecl {{.*}} struct Struct definition<br>
+// CHECK: | |-CXXMethodDecl {{.*}} simple_method 'void () __attribute__((thiscall))'<br>
+// CHECK: | |-CXXMethodDecl {{.*}} virtual_method 'void () __attribute__((thiscall))' virtual<br>
----------------<br>
zturner wrote:<br>
> I think this is wrong.  If I compile this file myself and look at the debug info, the debug info says it has cdecl calling convention.  <br>
> <br>
> <br>
> ```<br>
> D:\src\llvmbuild\ninja-x64>bin\llvm-pdbutil.exe dump -types foo.pdb | grep -C 5 simple_method<br>
>    0x106D | LF_METHODLIST [size = 20]<br>
>             - Method [type = 0x106B, vftable offset = -1, attrs = public compiler-generated]<br>
>             - Method [type = 0x106C, vftable offset = -1, attrs = public compiler-generated]<br>
>    0x106E | LF_FIELDLIST [size = 152]<br>
>             - LF_VFUNCTAB type = 0x1056<br>
>             - LF_ONEMETHOD [name = `simple_method`]<br>
>               type = 0x1059, vftable offset = -1, attrs = public<br>
>             - LF_ONEMETHOD [name = `virtual_method`]<br>
>               type = 0x1059, vftable offset = 0, attrs = public intro virtual<br>
>             - LF_ONEMETHOD [name = `static_method`]<br>
>               type = 0x105A, vftable offset = -1, attrs = public static<br>
> <br>
> D:\src\llvmbuild\ninja-x64>bin\llvm-pdbutil.exe dump -types -type-index=0x1059 foo.pdb<br>
> <br>
> <br>
>                      Types (TPI Stream)<br>
> ============================================================<br>
>   Showing 1 records.<br>
>    0x1059 | LF_MFUNCTION [size = 28]<br>
>             return type = 0x0003 (void), # args = 0, param list = 0x1011<br>
>             class type = 0x1057, this type = 0x1058, this adjust = 0<br>
>             calling conv = cdecl, options = None<br>
> ```<br>
> <br>
> Also, if I use clang-cl and pass the `-ast-dump` option, it will look like this:<br>
> <br>
> ```<br>
> | |-CXXMethodDecl 0x23869a32c38 <line:2:3, col:25> col:8 simple_method 'void ()'<br>
> | | `-CompoundStmt 0x23869a336f0 <col:24, col:25><br>
> ```<br>
> <br>
> So it doesn't have `__attribute__((thiscall))` as you do in the test.  Could you double check this?<br>
Hm... I tested it on x86, and I had there a "thiscall" attribute. It seems that on x64 it is not so. I think it's because the struct contains no fields and the methods doesn't use them - they don't need a "this" pointer. But I can't understand, why is there still "thiscall" on x86. Unfortunately, I can check it only two weeks later, but thanks for catching that!</blockquote><div dir="auto">Ahh that explains it.  X64 just uses one calling convention and doesn’t distinguish between thiscall (the compiler will even ignore the keyword).  I think we can just remove this from the test and it should be fine </div><div dir="auto"><br></div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:669-672<br>
+    MemberFunctionRecord mfr;<br>
+    llvm::cantFail(<br>
+        TypeDeserializer::deserializeAs<MemberFunctionRecord>(cvt, mfr));<br>
+    return CreateFunctionType(mfr.ArgumentList, mfr.ReturnType, mfr.CallConv);<br>
----------------<br>
zturner wrote:<br>
> I think there is a problem here.  <br>
I can't understand, why?..<br>
<br>
<br>
Repository:<br>
  rLLDB LLDB<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D56126/new/" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D56126/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D56126" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D56126</a><br>
<br>
<br>
<br>
</blockquote></div></div>
</blockquote></div>