[PATCH] D152914: [Draft] Make __builtin_cpu builtins target-independent

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:903-907
+// Load of a value provided by the system library at a fixed address. Used for
+// accessing things like HWCAP word provided by GLIBC.
+def int_fixed_addr_ld
+    : DefaultAttrsIntrinsic<[llvm_i32_ty], [llvm_i32_ty],
+                            [IntrInaccessibleMemOnly, ImmArg<ArgIndex<0>>]>;
----------------
arsenm wrote:
> nemanjai wrote:
> > arsenm wrote:
> > > nemanjai wrote:
> > > > arsenm wrote:
> > > > > From this description I don't understand what this is supposed to do. What does the input mean? Why does this use an i32 immarg and not a pointer? Why is the result only i32?
> > > > That is fair enough. The description is fairly vague. I can try to improve it as per below. The parameter for this is not a pointer (i.e. not an address). It is an immediate that represents the index into the enumeration of values that are provided at a fixed address. The back end is then free to produce the actual fixed address and the load itself.
> > > > The choice for the result type was admittedly arbitrary - on PPC, the values provided by GLIBC are 32-bit words.
> > > > 
> > > > Proposed comment describing this intrinsic:
> > > > ```
> > > > // This intrinsic is provided to allow back ends to emit load
> > > > // instructions that load a value from a fixed address. The
> > > > // parameter to the intrinsic is not an address, but an
> > > > // immediate index into an enumeration that contains the
> > > > // union of all such values available on all back ends.
> > > > // An example is the HWCAP/HWCAP2/CPUID words
> > > > // provided by GLIBC on PowerPC to allow fast access
> > > > // to commonly used parts of AUXV. These are provided
> > > > // at a fixed offset into the TCB (accessible through the
> > > > // thread pointer).
> > > > ```
> > > This is baking an a very target specific implementation of device identification. Could you redefine this as something more abstract? Like returns a device ID integer, or a bool that some int input is supported?
> > The idea is for this to not be restricted to device ID at all, but to be used for any values that reside in a fixed address for the compiler. For example, `STACK_GUARD` can be one of the values. How about if the intrinsic returns any integer type (or maybe any type)?
> > My thinking is that there may be need in the future for various things to be loaded from fixed addresses. The list of possible things that can be loaded this way would be a union of what all targets want and only specific values would make sense on each target.
> But you're assuming there's a fixed address this can be loaded from, and not a read from a special register or some other mechanism
Oh, well sure. My intent was just for those things that are in memory at a fixed address. But I suppose you're right, there is probably not a fundamental reason to restrict it to things at fixed addresses.

What I am trying to avoid here is defining an intrinsic that takes a specific value and returns a `bool` (i.e. the intrinsic version of `__builtin_cpu_{supports|is}`. The features/cpus will be different for every target and mapping features to integers is also target specific. So this was an attempt to implement an intrinsic that gets a value (bit vector if you will) that the target can choose how to lower and what to do with the value (i.e. how to mask it).

I suppose each target can define their own intrinsics for accessing CPU identification information since each target has to provide code generation for them anyway.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6787
   }
+  case Intrinsic::fixed_addr_ld: {
+    SDValue Chain = getRoot();
----------------
This should check `useLoadFixedAddr()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152914/new/

https://reviews.llvm.org/D152914