Re: [PATCH] D20709: Support ARM subtarget feature +long64

2016-06-01 Thread Richard Smith via cfe-commits
rsmith added a subscriber: rsmith.
rsmith added a comment.

The summary and content of this patch are very different.

If you have an ABI variant that any language should be able to target, that 
should be controlled by the triple and related flags, not by a RenderScript 
language option.

If you want to introduce RenderScript as a supported language in Clang, that 
may well be reasonable, but that's a very different discussion.

Normally, we don't want to have ABI variants in clang that only a single 
language can target. There doesn't seem to be any fundamental reason why you 
couldn't compile C++ code for the RenderScript ABI, for instance, so tying the 
ABI to the language doesn't seem like the right approach.


http://reviews.llvm.org/D20709



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20709: Support ARM subtarget feature +long64

2016-06-01 Thread Pirama Arumuga Nainar via cfe-commits
pirama updated this revision to Diff 59294.
pirama removed a reviewer: rsmith.
pirama added a comment.
Herald added subscribers: danalbert, tberghammer.

Add a RenderScript langopt and updated to change long's size and alignment in
TargetInfo::adjust().


http://reviews.llvm.org/D20709

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Types.def
  include/clang/Frontend/FrontendOptions.h
  lib/Basic/TargetInfo.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  test/CodeGen/renderscript.c

Index: test/CodeGen/renderscript.c
===
--- /dev/null
+++ test/CodeGen/renderscript.c
@@ -0,0 +1,5 @@
+// RUN: %clang -target arm-linux-androideabi -DLONG_SIZE_AND_ALIGN=4 -fsyntax-only %s
+// RUN: %clang -target arm-linux-androideabi -x renderscript -DLONG_SIZE_AND_ALIGN=8 -fsyntax-only %s
+
+_Static_assert(sizeof(long) == LONG_SIZE_AND_ALIGN, "sizeof long is wrong");
+_Static_assert(_Alignof(long) == LONG_SIZE_AND_ALIGN, "alignof long is wrong");
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -735,6 +735,7 @@
   case IK_PreprocessedObjCXX:
   case IK_AST:
   case IK_LLVM_IR:
+  case IK_Renderscript:
 // We can't do anything with these.
 return;
   }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1242,6 +1242,7 @@
   .Case("objective-c++-header", IK_ObjCXX)
   .Cases("ast", "pcm", IK_AST)
   .Case("ir", IK_LLVM_IR)
+  .Case("renderscript", IK_Renderscript)
   .Default(IK_None);
 if (DashX == IK_None)
   Diags.Report(diag::err_drv_invalid_value)
@@ -1445,6 +1446,9 @@
 case IK_PreprocessedObjCXX:
   LangStd = LangStandard::lang_gnucxx98;
   break;
+case IK_Renderscript:
+  LangStd = LangStandard::lang_c99;
+  break;
 }
   }
 
@@ -1487,6 +1491,8 @@
   Opts.CUDA = IK == IK_CUDA || IK == IK_PreprocessedCuda ||
   LangStd == LangStandard::lang_cuda;
 
+  Opts.Renderscript = IK == IK_Renderscript;
+
   // OpenCL and C++ both have bool, true, false keywords.
   Opts.Bool = Opts.OpenCL || Opts.CPlusPlus;
 
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -318,6 +318,14 @@
 FloatFormat = ::APFloat::IEEEsingle;
 LongDoubleFormat = ::APFloat::IEEEquad;
   }
+
+  if (Opts.Renderscript) {
+// In RenderScript, long types are always 8 bytes wide and aligned to
+// 8-byte boundary.
+if (PointerWidth == 32) {
+  LongWidth = LongAlign = 64;
+}
+  }
 }
 
 bool TargetInfo::initFeatureMap(
Index: include/clang/Frontend/FrontendOptions.h
===
--- include/clang/Frontend/FrontendOptions.h
+++ include/clang/Frontend/FrontendOptions.h
@@ -75,7 +75,8 @@
   IK_CUDA,
   IK_PreprocessedCuda,
   IK_AST,
-  IK_LLVM_IR
+  IK_LLVM_IR,
+  IK_Renderscript
 };
 
   
Index: include/clang/Driver/Types.def
===
--- include/clang/Driver/Types.def
+++ include/clang/Driver/Types.def
@@ -53,6 +53,7 @@
 TYPE("objective-c++-cpp-output", PP_ObjCXX,INVALID, "mii",   "u")
 TYPE("objc++-cpp-output",PP_ObjCXX_Alias, INVALID,  "mii",   "u")
 TYPE("objective-c++",ObjCXX,   PP_ObjCXX,   "mm","u")
+TYPE("renderscript", Renderscript, PP_C,"rs","u")
 
 // C family input files to precompile.
 TYPE("c-header-cpp-output",  PP_CHeader,   INVALID, "i", "p")
Index: include/clang/Basic/LangOptions.def
===
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -185,6 +185,7 @@
 LANGOPT(OpenMP, 1, 0, "OpenMP support")
 LANGOPT(OpenMPUseTLS  , 1, 0, "Use TLS for threadprivates or runtime calls")
 LANGOPT(OpenMPIsDevice, 1, 0, "Generate code only for OpenMP target device")
+LANGOPT(Renderscript  , 1, 0, "RenderScript")
 
 LANGOPT(CUDAIsDevice  , 1, 0, "compiling for CUDA device")
 LANGOPT(CUDAAllowVariadicFunctions, 1, 0, "allowing variadic functions in CUDA device code")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20709: Support ARM subtarget feature +long64

2016-06-01 Thread Pirama Arumuga Nainar via cfe-commits
pirama added a reviewer: rsmith.
pirama added a comment.

Adding Richard to reviewers as the planned direction of this patch directly 
relates to the Frontend.


http://reviews.llvm.org/D20709



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20709: Support ARM subtarget feature +long64

2016-05-31 Thread Pirama Arumuga Nainar via cfe-commits
pirama added a comment.

I looked at how LangOpts and TargetInfo are handled and found a way we can 
achieve 64-bit longs without a Target feature (and restrict it just to 
RenderScript!).  This would be via TargetInfo::adjust().  I'll abandon the 
sibling patch to LLVM as we should be able to do this just in Clang.


http://reviews.llvm.org/D20709



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20709: Support ARM subtarget feature +long64

2016-05-31 Thread Renato Golin via cfe-commits
rengolin added a comment.

In http://reviews.llvm.org/D20709#443950, @srhines wrote:

> Correct. This is only used by RenderScript, and unfortunately can't be done 
> any differently. We had hoped to just predicate this with our own LangOpt 
> (the patch that adds the RenderScript LangOpt is coming soon), but 
> unfortunately, at the point where LangOpts are being parsed, the target 
> machine already has the size and alignment of long specified (and it can't 
> change).


Hi Stephen,

GCC uses -mabi=ilp32 or -mabi=lp64 for AArch64 to change the ABI, you could do 
the same on Renderscript.

If this is how Renderscript *always* operate, than having a "renderscript" ABI 
would make even more sense.

cheers,
--renato


http://reviews.llvm.org/D20709



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20709: Support ARM subtarget feature +long64

2016-05-30 Thread Stephen Hines via cfe-commits
srhines added a comment.

In http://reviews.llvm.org/D20709#443536, @kristof.beyls wrote:

> Hi Pirama,
>
> My understanding is that this introduces (yet another) ARM 32 bit ABI variant 
> - in this case with longs being 64 bit.
>  My understanding is also that this is the ABI that is used in Renderscript, 
> and this patch helps to remove local patches that currently live in the 
> Renderscript toolchain downstream only.


Correct. This is only used by RenderScript, and unfortunately can't be done any 
differently. We had hoped to just predicate this with our own LangOpt (the 
patch that adds the RenderScript LangOpt is coming soon), but unfortunately, at 
the point where LangOpts are being parsed, the target machine already has the 
size and alignment of long specified (and it can't change).

> I think it's good not to need downstream patches for a Renderscript toolchain.


Same here, although these patches seem so small that I don't want to introduce 
too much trouble in upstream. If there isn't a nice way to clean this up, I 
would be fine with this amount of divergence, since it really doesn't impact 
our ability to update LLVM (and again, the feature is only used by 
RenderScript).

> In effect, this introduces another abi variant.

>  I'm wondering if the abi variant should be controlled using a triple, just 
> like e.g. the hard float vs. soft float abi variants also get controlled via 
> a triple?


We can't unfortunately control this via a separate triple because RenderScript 
has (and always) will use the ARM triple. This really only affects the frontend 
for RenderScript, since RS doesn't allow the full libc runtime. That is why the 
LLVM side of this patch has no tests (because there is nothing there, and we 
only need the option recognized or the backend generates warnings about an 
unsupported flag - perhaps there is a better way to suppress that, but at the 
time that I fixed this years ago, I think my only option was to add it in both 
places).

> Tim, do you happen to have insights on this?

> 

> Thanks,

> 

> Kristof





http://reviews.llvm.org/D20709



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20709: Support ARM subtarget feature +long64

2016-05-30 Thread Kristof Beyls via cfe-commits
kristof.beyls added a comment.

Hi Pirama,

My understanding is that this introduces (yet another) ARM 32 bit ABI variant - 
in this case with longs being 64 bit.
My understanding is also that this is the ABI that is used in Renderscript, and 
this patch helps to remove local patches that currently live in the 
Renderscript toolchain downstream only.

I think it's good not to need downstream patches for a Renderscript toolchain.

In effect, this introduces another abi variant.
I'm wondering if the abi variant should be controlled using a triple, just like 
e.g. the hard float vs. soft float abi variants also get controlled via a 
triple?
Tim, do you happen to have insights on this?

Thanks,

Kristof


http://reviews.llvm.org/D20709



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D20709: Support ARM subtarget feature +long64

2016-05-26 Thread Pirama Arumuga Nainar via cfe-commits
pirama created this revision.
pirama added a reviewer: kristof.beyls.
pirama added subscribers: srhines, cfe-commits.
Herald added subscribers: rengolin, aemerson.

Set alignment and width of long datatype to be 64-bits if the ARM
subtarget feature +long64 is set.

http://reviews.llvm.org/D20709

Files:
  lib/Basic/Targets.cpp
  test/CodeGen/arm-types.c

Index: test/CodeGen/arm-types.c
===
--- /dev/null
+++ test/CodeGen/arm-types.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple arm -DLONG_SIZE_AND_ALIGN=4
+// RUN: %clang_cc1 -triple arm -target-feature +long64 -DLONG_SIZE_AND_ALIGN=8
+
+_Static_assert(sizeof(long) == LONG_SIZE_AND_ALIGN, "sizeof long is wrong");
+_Static_assert(_Alignof(long) == LONG_SIZE_AND_ALIGN, "alignof long is wrong");
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -4955,6 +4955,8 @@
 Unaligned = 0;
   } else if (Feature == "+fp16") {
 HW_FP |= HW_FP_HP;
+  } else if (Feature == "+long64") {
+LongWidth = LongAlign = 64;
   }
 }
 HW_FP &= ~HW_FP_remove;


Index: test/CodeGen/arm-types.c
===
--- /dev/null
+++ test/CodeGen/arm-types.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple arm -DLONG_SIZE_AND_ALIGN=4
+// RUN: %clang_cc1 -triple arm -target-feature +long64 -DLONG_SIZE_AND_ALIGN=8
+
+_Static_assert(sizeof(long) == LONG_SIZE_AND_ALIGN, "sizeof long is wrong");
+_Static_assert(_Alignof(long) == LONG_SIZE_AND_ALIGN, "alignof long is wrong");
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -4955,6 +4955,8 @@
 Unaligned = 0;
   } else if (Feature == "+fp16") {
 HW_FP |= HW_FP_HP;
+  } else if (Feature == "+long64") {
+LongWidth = LongAlign = 64;
   }
 }
 HW_FP &= ~HW_FP_remove;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits