For RenderScript, set alignment and width of long to 64-bits

[pirama]

Summary

This patch sets width and alignment of 'long' to 64-bits if the
Renderscript LangOpt (also added as part of this patch) is set.

This patch also makes the LangOpt visible by adding 'renderscript' as a
new language type that can be specified as an option to '-x'.

[kristof.beyls]

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

[srhines]

In 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

[rengolin]

In 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

[pirama]

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.

[pirama]

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

[pirama]

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

[rsmith]

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.

[srhines]

After talking to Richard this afternoon on IRC, we are going with a slightly different plan here. We will create two new triples (renderscript32-*-* and renderscript64-*-*), which are direct subclasses of the general Android ARM32 and ARM64 targets. In this case, we can use all of the existing ABI info we already have built up for these targets (and override longs to be 64-bit on the renderscript32-*-* target). Additionally, the renderscript targets will also set the triple name back to the ARM-related triple, since we can't break compatibility for generating RenderScript code that is expected to embed the ARM triples in the bitcode (mostly ignored, but might be validated).

One thing that didn't come up in the IRC discussion, but we probably still need a LangOpt for RenderScript. There is a coming patch that adds a single attribute (for "kernel"), that has error checking to ensure that it is only used by our RenderScript targets. I assume that the cleanest way to retain that diagnostic (and get the last of these divergent patches in) will be to still key off of a LangOpt. If you think it is preferable to check against a target instead, I can change the code to do that, but I figured this is more consistent with how other attributes work.

[rsmith]

In D20709#446595, @srhines wrote:

There is a coming patch that adds a single attribute (for "kernel"), that has error checking to ensure that it is only used by our RenderScript targets. I assume that the cleanest way to retain that diagnostic (and get the last of these divergent patches in) will be to still key off of a LangOpt. If you think it is preferable to check against a target instead, I can change the code to do that, but I figured this is more consistent with how other attributes work.

We discussed this again today, and our preferred way forward is:

1. We have new targets for 32-bit and 64-bit renderscript as described here. The new targets are exactly the same as the relevant ARM targets except that they force the size and alignment of long to 8. (We may want to consider also making these targets imply -emit-llvm, especially if our notional model for the target is that its "machine code" is LLVM IR with an ARM triple.)

2. We have a new LangOption and InputKind for renderscript, which can be specified via -x renderscript and is implied by a .rs file extension. This LangOption controls the existence of the "kernel" attribute, and some renderscript-specific implicit conversion rules.

... and we treat (1) and (2) as orthogonal concerns (much like the SPIR target and OpenCL language); you can compile plain C or C++ to the renderscript target if you want, or request the renderscript language rules while targeting x86, even though these are not likely to be particularly useful configurations in practice.

[pirama]

D21333 and D21334 add the RenderScript triple and modify 'long' based off the triple.

r272317, r272342, r272438 take care of the LangOpt and handling of '-x renderscript' and '.rs' extensions.

[pirama]

Abandoning this revision. Patches for an alternative approach are listed above.

include/clang/Basic/LangOptions.def

 LANGOPT(OpenCLVersion     , 32, 0, "OpenCL version")
 LANGOPT(NativeHalfType    , 1, 0, "Native half type support")
 LANGOPT(NativeHalfArgsAndReturns, 1, 0, "Native half args and returns")
 LANGOPT(HalfArgsAndReturns, 1, 0, "half args and returns")
 LANGOPT(CUDA              , 1, 0, "CUDA")
 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")
 LANGOPT(CUDAHostDeviceConstexpr, 1, 1, "treating unattributed constexpr functions as __host__ __device__")
 LANGOPT(CUDADeviceFlushDenormalsToZero, 1, 0, "flushing denormals to zero")
 
 LANGOPT(SizedDeallocation , 1, 0, "enable sized deallocation functions")
 LANGOPT(ConceptsTS , 1, 0, "enable C++ Extensions for Concepts")

include/clang/Driver/Types.def

 TYPE("objective-c-cpp-output",   PP_ObjC,      INVALID,         "mi",    "u")
 TYPE("objc-cpp-output",          PP_ObjC_Alias, INVALID,        "mi",    "u")
 TYPE("objective-c",              ObjC,         PP_ObjC,         "m",     "u")
 TYPE("c++-cpp-output",           PP_CXX,       INVALID,         "ii",    "u")
 TYPE("c++",                      CXX,          PP_CXX,          "cpp",   "u")
 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")
 TYPE("c-header",                 CHeader,      PP_CHeader,      "h",     "pu")
 TYPE("cl-header",                CLHeader,     PP_CHeader,      "h",     "pu")
 TYPE("objective-c-header-cpp-output", PP_ObjCHeader, INVALID,   "mi",    "p")
 TYPE("objective-c-header",       ObjCHeader,   PP_ObjCHeader,   "h",     "pu")
 TYPE("c++-header-cpp-output",    PP_CXXHeader, INVALID,         "ii",    "p")

include/clang/Frontend/FrontendOptions.h

enum InputKind {
   IK_PreprocessedC,
   IK_PreprocessedCXX,
   IK_PreprocessedObjC,
   IK_PreprocessedObjCXX,
   IK_OpenCL,
   IK_CUDA,
   IK_PreprocessedCuda,
   IK_AST,
-  IK_LLVM_IR
+  IK_LLVM_IR,
+  IK_Renderscript
 };
 
   
 /// \brief An input file for the front end.
 class FrontendInputFile {
   /// \brief The file name, or "-" to read from standard input.
   std::string File;
 

lib/Basic/TargetInfo.cpp

if (Opts.OpenCL) {
 
     IntMaxType = SignedLongLong;
     Int64Type = SignedLong;
 
     HalfFormat = &llvm::APFloat::IEEEhalf;
     FloatFormat = &llvm::APFloat::IEEEsingle;
     LongDoubleFormat = &llvm::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(
     llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
     const std::vector<std::string> &FeatureVec) const {
   for (const auto &F : FeatureVec) {
     StringRef Name = F;
     // Apply the feature via the target.

lib/Frontend/CompilerInvocation.cpp

DashX = llvm::StringSwitch<InputKind>(A->getValue())
       .Case("objc++-cpp-output", IK_PreprocessedObjCXX)
       .Case("c-header", IK_C)
       .Case("cl-header", IK_OpenCL)
       .Case("objective-c-header", IK_ObjC)
       .Case("c++-header", IK_CXX)
       .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)
         << A->getAsString(Args) << A->getValue();
   }
 
   // '-' is the default input if none is given.
   std::vector<std::string> Inputs = Args.getAllArgValues(OPT_INPUT);
case IK_PreprocessedObjC:
         LangStd = LangStandard::lang_gnu11;
       break;
     case IK_CXX:
     case IK_PreprocessedCXX:
     case IK_ObjCXX:
     case IK_PreprocessedObjCXX:
       LangStd = LangStandard::lang_gnucxx98;
       break;
+    case IK_Renderscript:
+      LangStd = LangStandard::lang_c99;
+      break;
     }
   }
 
   const LangStandard &Std = LangStandard::getLangStandardForKind(LangStd);
   Opts.LineComment = Std.hasLineComments();
   Opts.C99 = Std.isC99();
   Opts.C11 = Std.isC11();
   Opts.CPlusPlus = Std.isCPlusPlus();
if (Opts.OpenCL) {
     Opts.DefaultFPContract = 1;
     Opts.NativeHalfType = 1;
     Opts.NativeHalfArgsAndReturns = 1;
   }
 
   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;
 
   // OpenCL has half keyword
   Opts.Half = Opts.OpenCL;
 
   // C++ has wchar_t keyword.
   Opts.WChar = Opts.CPlusPlus;

lib/Frontend/FrontendActions.cpp

void PrintPreambleAction::ExecuteAction() {
   case IK_Asm:
   case IK_PreprocessedC:
   case IK_PreprocessedCuda:
   case IK_PreprocessedCXX:
   case IK_PreprocessedObjC:
   case IK_PreprocessedObjCXX:
   case IK_AST:
   case IK_LLVM_IR:
+  case IK_Renderscript:
     // We can't do anything with these.
     return;
   }
 
   CompilerInstance &CI = getCompilerInstance();
   auto Buffer = CI.getFileManager().getBufferForFile(getCurrentFile());
   if (Buffer) {
     unsigned Preamble =
         Lexer::ComputePreamble((*Buffer)->getBuffer(), CI.getLangOpts()).first;
     llvm::outs().write((*Buffer)->getBufferStart(), Preamble);
   }
 }

test/CodeGen/renderscript.c

+// 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");