pmatos added a comment.
I will be splitting the part enabling the target feature through clang into a
separate revision as suggested by @tlively
================
Comment at: clang/lib/Basic/Targets/WebAssembly.cpp:43
.Case("exception-handling", HasExceptionHandling)
+ .Case("reference-types", HasReferenceTypes)
.Case("bulk-memory", HasBulkMemory)
----------------
tlively wrote:
> I would split the target-features changes out into a separate patch that we
> can land right away. There is utility in having the feature flag available to
> be put into the target-features section even if LLVM cannot itself emit
> reference types usage yet.
Sure - I will create a separate revision for this bit then.
================
Comment at: clang/lib/Basic/Targets/WebAssembly.cpp:183
+ HasReferenceTypes = true;
+ resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:256");
+ continue;
----------------
tlively wrote:
> It would be good to have a comment about what is different in this data
> layout string. Also, is it really necessary to have a different data layout
> string when reference types are enabled?
ni will define 256 as a non-integral address space. This is required for
externref but I don't think it hurts to have it even if reference types is
disabled but it feels cleaner to not have it if not needed.
================
Comment at: llvm/include/llvm/CodeGen/ValueTypes.td:169
def exnref: ValueType<0, 134>; // WebAssembly's exnref type
+def externref: ValueType<0, 135>; // WebAssembly's externref type
def token : ValueType<0 , 248>; // TokenTy
----------------
tlively wrote:
> Do we need `funcref` here as well, or do we just use `externref` everywhere
> and infer separate `funcref` types later?
That's the plan for now. I think I haven't seen the use of having funcref here
as well so I haven't added it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66035/new/
https://reviews.llvm.org/D66035
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
