[PATCH] D34377: Support MS builtins using 'long' on Darwin/LP64

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 20 14:25:52 PDT 2017


rnk added inline comments.


================
Comment at: lib/AST/ASTContext.cpp:8551
+      break;
+    }
     case 'W':
----------------
majnemer wrote:
> bruno wrote:
> > bruno wrote:
> > > rnk wrote:
> > > > compnerd wrote:
> > > > > I agree with @majnemer.  Why not base this on the Int64Type?
> > > > I'd suggest this code:
> > > >   IsSpecialLong = true;
> > > >   // Use "long" if is 32 bits. This prefix is used by intrinsics that need 32-bit types on LP64 platforms, but need to use "long" in the prototype on LLP64 platforms like Win64.
> > > >   if (Context.getTargetInfo().getLongWidth() == 32)
> > > >     HowLong = 1;
> > > >   break;
> > > See below.
> > I tried something similar before, but I get two tests failing CodeGen/ms-intrinsics-rotations.c and CodeGen/pr27892.c. Your suggestion hits the same failing tests. Both fails because of the Linux issue mentioned above: i32 codegen where i64 is expected. Of course I could improve the condition to handle Linux, but at that point I just thing it's better to use Darwin, which is what the fix is towards anyway. Additional ideas?
> I don't think we should sweep this under the rug just because there are some test failures. There is probably some latent bug worth investigating.
I think I remember answer a question for Albert during his internship, and I said something like "they should stay longs", so he added those tests. Thinking about it now, those test should be changed to expect 32-bit ints.


https://reviews.llvm.org/D34377





More information about the cfe-commits mailing list