[PATCH] D105423: Add support for Opaque as a LowLevelType

Paulo Matos via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 7 23:44:44 PDT 2021


pmatos added a comment.

In D105423#2862135 <https://reviews.llvm.org/D105423#2862135>, @tlively wrote:

> In D105423#2862011 <https://reviews.llvm.org/D105423#2862011>, @arsenm wrote:
>
>> In D105423#2862010 <https://reviews.llvm.org/D105423#2862010>, @pmatos wrote:
>>
>>> In D105423#2861795 <https://reviews.llvm.org/D105423#2861795>, @arsenm wrote:
>>>
>>>> I'm not sure what an opaque type means. Do you have an IR sample?
>>>
>>> Sure - for example, from how we represent externrefs (opaque object references in WebAssembly):
>>>
>>>   %extern = type opaque
>>>   %externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral
>>>   @externref_global = local_unnamed_addr addrspace(2) global %externref undef
>>>   
>>>   define %externref @return_externref_global() {
>>>     ;; this generates a global.get of @externref_global
>>>     %ref = load %externref, %externref addrspace(2)* @externref_global
>>>     ret %externref %ref
>>>   }
>>
>> But here it's just a pointer with an address space and you aren't actually using the opaque type as a value. Why can't you just treat this as a regular pointer?
>
> IIUC, the "opaque" type (in the sense of this patch) here is %externref, which lowers to a zero-sized (i.e. no bit representation) MVT. So it is a regular pointer except that it has zero size, which requires special care here and elsewhere in https://reviews.llvm.org/D104797. From @pmatos's latest comments on that other revision, perhaps an alternative to this patch would be to allow scalar LLTs to have zero size?

Sorry @arsenm ,  I should have explained it better, as @tlively did instead of posting the example and leaving for the day.

In any case, as @Tlively said the MVT for `externref` is zero-sized. See https://github.com/llvm/llvm-project/blob/0fd5e7b2d8ca4ed46d76187feb4b903ed0a3ea75/llvm/include/llvm/Support/MachineValueType.h#L1049

@tlively actually your suggestion of making the opaque type a scalar type of size zero was my first attempt : Failed! Why? The idea what that I would differentiate between a valid scalar and an opaque type depending on if its value was zero. So, I actually implemented an initial patch where I had something like this:

  bool IsOpaque() { return isValid() && !IsPointer && !IsVector && getSizeInBits() == 0; }
  bool IsScalar() { return isValid() && !IsPointer && !IsVector && getSizeInBits() != 0; }

I also created a constructor to construct an opaque type as a scalar type of size zero. But this didn't work because the implementation of `getSizeInBits()` itself calls `IsScalar()`. So this resulted in an infinite recursive call. I then decided to create its own type for Opaque as in this patch.

However, I am looking at the code again and notice that actually, I could call in `IsOpaque()` the function `getScalarSizeInBits()` instead of `getSizeInBits()`. That could work. I will get this patch redone,  test, and submit it as an alternative revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105423



More information about the llvm-commits mailing list