[PATCH] D43630: [Driver] Fix search paths on x32

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

This should probably have some tests. It looks like there are some existing x32 
tests in test/Driver/linux-header-search.cpp and test/Driver/cross-linux.c that 
might be relevant for updating.


Repository:
  rC Clang

https://reviews.llvm.org/D43630



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

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

LGTM assuming we are convinced for now that finding the rethrow block from the 
CatchSwitch will work. Does this need to wait until after 
https://reviews.llvm.org/D43746 or other CLs from that chain, or does it matter?


Repository:
  rC Clang

https://reviews.llvm.org/D44931



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

[PATCH] D39218: [WebAssembly] Include libclang_rt.builtins in the standard way

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lib/Driver/ToolChain.cpp:318
+  else
+OSLibName = getOS();
   llvm::sys::path::append(Path, "lib", OSLibName);

Is this logic intended to replace what was removed from CommonArgs.cpp? Should 
there be an assert here too?


https://reviews.llvm.org/D39218



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

[PATCH] D39218: [WebAssembly] Include libclang_rt.builtins in the standard way

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Driver/ToolChain.cpp:318
+  else
+OSLibName = getOS();
   llvm::sys::path::append(Path, "lib", OSLibName);

sbc100 wrote:
> dschuff wrote:
> > Is this logic intended to replace what was removed from CommonArgs.cpp? 
> > Should there be an assert here too?
> Just didn't see the point of that assert.  Can you see what the intention 
> might be?  I don't see why AddRunTimeLibs() should be callable for any/all 
> triples, do you?  I would have had to add  llvm::Triple::Unknown to the list 
> of supported OSs, which seemed strange.
I assume it was just because different OSes have different conventions for the 
rtlib, and if the OS is unknown and/or there's no particular support, then 
there might be a bug. But OTOH it doesn't seem bad to have some reasonable 
default either. For that matter it also seems a little weird that now we are 
letting the binary format be the determining thing for the Compiler-RT path. 
But I guess the real issue is that none of the OS, arch, or binary format is 
really the determining thing; it's really the toolchain/SDK or distribution of 
the compiler that determines what the path should be, and that can be affected 
by a lot of other things (e.g. is it the "system" compiler or not, is it a 
cross compiler, etc). So... yeah I think having this as a default makes as much 
sense as asserting, and when someone needs to add support for their 
distribution, then I suppose it's on them to refactor as needed and keep the 
tests working.


https://reviews.llvm.org/D39218



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

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: include/clang/Basic/BuiltinsWebAssembly.def:38
+// Atomic wait and wake.
+BUILTIN(__builtin_wasm_atomic_wait_i32, "Uii*iLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wait_i64, "UiLLi*LLiLLi", "n")

So this means that the signature is basically `unsigned int 
__builtin_wasm_atomic_wait_i32(int *, int, long long)`? We should maybe make it 
`int __builtin_wasm_atomic_wait_i32(const unsigned char *, int, unsigned long 
long)`. Returning int so that you could define a C enum with the possible 
return values and compare without type coercion; unsigned char * so that it 
aliases with everything (i.e. a byte ptr), and unsigned long long since a 
negative relative timeout isn't meaningful(?). Not sure whether we should use 
int or unsigned int as the expected value, can't think of any particular reason 
right now to use one or the other.

Likewise with the other signatures.


Repository:
  rC Clang

https://reviews.llvm.org/D49396



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

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: include/clang/Basic/BuiltinsWebAssembly.def:38
+// Atomic wait and wake.
+BUILTIN(__builtin_wasm_atomic_wait_i32, "Uii*iLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wait_i64, "UiLLi*LLiLLi", "n")

aheejin wrote:
> dschuff wrote:
> > So this means that the signature is basically `unsigned int 
> > __builtin_wasm_atomic_wait_i32(int *, int, long long)`? We should maybe 
> > make it `int __builtin_wasm_atomic_wait_i32(const unsigned char *, int, 
> > unsigned long long)`. Returning int so that you could define a C enum with 
> > the possible return values and compare without type coercion; unsigned char 
> > * so that it aliases with everything (i.e. a byte ptr), and unsigned long 
> > long since a negative relative timeout isn't meaningful(?). Not sure 
> > whether we should use int or unsigned int as the expected value, can't 
> > think of any particular reason right now to use one or the other.
> > 
> > Likewise with the other signatures.
> > Returning int so that you could define a C enum with the possible return 
> > values and compare without type coercion;
> Done.
> 
> > unsigned char * so that it aliases with everything (i.e. a byte ptr),
> From this pointer a value will be loaded and compared with the expected 
> value, which is an int. Shouldn't this be an int pointer then? Not sure why 
> it should alias with a byte ptr.
> 
> > and unsigned long long since a negative relative timeout isn't 
> > meaningful(?).
> [[ 
> https://github.com/WebAssembly/threads/blob/master/proposals/threads/Overview.md#wait
>  | Timeouts can be negative ]], in which case it never expires. The wake 
> count of `atomics.wake` builtin can be negative too, in which case it waits 
> for all waiters.
> 
> > Not sure whether we should use int or unsigned int as the expected value, 
> > can't think of any particular reason right now to use one or the other.
> We didn't impose any restrictions other than it is an int in the spec, so I 
> think it should be a (signed) int?
Oh you're right, I misread the spec. So int * and signed timeout is fine. For 
the expected value (and the pointer type) it could still be either signed or 
unsigned because at the wasm level there's no distinction. but signed int seems 
fine as it is.


Repository:
  rC Clang

https://reviews.llvm.org/D49396



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

[PATCH] D41923: [WebAssembly] Remove `-allow-undefined-file wasm.syms` from linker args

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

The code LGTM. If there are objections to the scheme overall they can go in 
tool-conventions.


Repository:
  rC Clang

https://reviews.llvm.org/D41923



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

[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lib/Basic/Targets/WebAssembly.h:108
   IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final 
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.

I think we want least16_t to still be short, no? We do still support 16-bit 
shorts, so my interpretation is that the smallest type with width of at least 
16 should still be 16.


Repository:
  rC Clang

https://reviews.llvm.org/D41941



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

[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lib/Basic/Targets/WebAssembly.h:108
   IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final 
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.

ncw wrote:
> ncw wrote:
> > dschuff wrote:
> > > I think we want least16_t to still be short, no? We do still support 
> > > 16-bit shorts, so my interpretation is that the smallest type with width 
> > > of at least 16 should still be 16.
> > ...is there a way to do that? I couldn't find any other archs that do it; 
> > it seems like the stdint.h that Clang provides requires least16_t to match 
> > fast16_t. I copied this from the AVR target, although maybe that doesn't 
> > support 16-bit at all.
> Sorry, now I see that AVR uses int because it has 16-bit ints...
> 
> There isn't any existing Clang target that uses 32-bit for fast16_t, so maybe 
> it's currently not possible within Clang's framework (or at least, not 
> without also fiddling with least16_t). `lib/Frontend/InitPreprocessor.cpp` 
> hardcodes some logic with sets them to be the same.
> 
> I can abandon this review if that's not acceptable collateral damage 
> (probably not, on reflection) - or could tweak InitPreprocessor.cpp and 
> stdint.h to be more flexible (might need more review if you don't "own" those 
> files?)
I think it's worth trying to fix InitPreprocessor.cpp/stdint.h to remove the 
assumption that those types are the same. We'll need to get broader review, so 
it will be slower than if we were just changing our own files, but that's OK. 
If you're up for doing that I'd be happy to help you find reviewers, or 
otherwise I can take a shot at it; now is a good time since we're taking a 
closer look at our ABIs more generally anyway.


Repository:
  rC Clang

https://reviews.llvm.org/D41941



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

[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lib/Basic/Targets/WebAssembly.h:108
   IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final 
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.

ncw wrote:
> dschuff wrote:
> > ncw wrote:
> > > ncw wrote:
> > > > dschuff wrote:
> > > > > I think we want least16_t to still be short, no? We do still support 
> > > > > 16-bit shorts, so my interpretation is that the smallest type with 
> > > > > width of at least 16 should still be 16.
> > > > ...is there a way to do that? I couldn't find any other archs that do 
> > > > it; it seems like the stdint.h that Clang provides requires least16_t 
> > > > to match fast16_t. I copied this from the AVR target, although maybe 
> > > > that doesn't support 16-bit at all.
> > > Sorry, now I see that AVR uses int because it has 16-bit ints...
> > > 
> > > There isn't any existing Clang target that uses 32-bit for fast16_t, so 
> > > maybe it's currently not possible within Clang's framework (or at least, 
> > > not without also fiddling with least16_t). 
> > > `lib/Frontend/InitPreprocessor.cpp` hardcodes some logic with sets them 
> > > to be the same.
> > > 
> > > I can abandon this review if that's not acceptable collateral damage 
> > > (probably not, on reflection) - or could tweak InitPreprocessor.cpp and 
> > > stdint.h to be more flexible (might need more review if you don't "own" 
> > > those files?)
> > I think it's worth trying to fix InitPreprocessor.cpp/stdint.h to remove 
> > the assumption that those types are the same. We'll need to get broader 
> > review, so it will be slower than if we were just changing our own files, 
> > but that's OK. If you're up for doing that I'd be happy to help you find 
> > reviewers, or otherwise I can take a shot at it; now is a good time since 
> > we're taking a closer look at our ABIs more generally anyway.
> uhh... if you could take a look that would be great! I've only got a limited 
> Wasm "budget" from my employer, and have to get back to customer work for 
> most of the rest of the week.
> 
> Was there an issue I seem to remember as well, where Wasm32 was using 
> "unsigned int" for __SIZE_TYPE__ instead of "unsigned long".
Yes, @sunfish is switching the wasm and asm.js ABIs to both use unsigned long.


Repository:
  rC Clang

https://reviews.llvm.org/D41941



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

[PATCH] D43681: [WebAssembly] Add exception handling option

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

This flag turns on CPU features (i.e. there's one for each new wasm feature 
proposal that has to be feature-detected), so I think this makes sense to be 
consistent with all the others. I could imagine enabling exceptions in the 
frontend but having some kind of emulation in the backend (like we do today for 
emscripten). More generally the semantics `-fno-exceptions` unfortunately 
doesn't exactly match the kind of behavior people typically want because it 
doesn't allow you to compile code that has try/catch/throw at all. In PNaCl and 
emscripten the default behavior is instead to compile that code but to lower it 
away and turn throw into abort. Also IIRC even with `-fno-exceptions` the code 
still has to allow exceptions to propagate which means you end up with invokes 
everywhere instead of calls, so you are still paying most of the costs of 
enabling EH. So that bit is a little bit complex and we'll probably want to 
think a bit more carefully about what options we want to have. But a machine 
flag for enabling the CPU feature makes sense to start.


Repository:
  rC Clang

https://reviews.llvm.org/D43681



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

[PATCH] D48443: [WIP] Add no-prototype attribute to prototype-less C functions

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1850
+FuncAttrs.addAttribute("no-prototype");
   AddAttributesFromFunctionProtoType(
   getContext(), FuncAttrs, Fn->getType()->getAs());

Would it make sense to put this in `AddAttributesFormFunctionProtoType` Does 
this have the same effect as that would? For that matter, what's the practical 
difference between this call and the one on line 1826?


Repository:
  rC Clang

https://reviews.llvm.org/D48443



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

[PATCH] D48443: [WIP] Add no-prototype attribute to prototype-less C functions

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1849
+  if (!AttrOnCallSite && !Fn->hasPrototype())
+FuncAttrs.addAttribute("no-prototype");
   AddAttributesFromFunctionProtoType(

aheejin wrote:
> Is there a reason why this is not something like 
> `llvm::Attribute::NoPrototype` like other attributes?
Target-independent attributes get enums, and target-specific attributes are 
just strings: https://llvm.org/docs/LangRef.html#attribute-groups 
We could potentially make this attribute target-independent if there is wider 
interest in removing the reliance on bare `(...)` signatures to signify 
prototypeless C functions. I would like to see more details of how we plan to 
handle this in the backend before I have any idea about what anyone else might 
thing.


Repository:
  rC Clang

https://reviews.llvm.org/D48443



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

[PATCH] D48443: [WIP] Add no-prototype attribute to prototype-less C functions

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1849
+  if (!AttrOnCallSite && !Fn->hasPrototype())
+FuncAttrs.addAttribute("no-prototype");
   AddAttributesFromFunctionProtoType(

dschuff wrote:
> aheejin wrote:
> > Is there a reason why this is not something like 
> > `llvm::Attribute::NoPrototype` like other attributes?
> Target-independent attributes get enums, and target-specific attributes are 
> just strings: https://llvm.org/docs/LangRef.html#attribute-groups 
> We could potentially make this attribute target-independent if there is wider 
> interest in removing the reliance on bare `(...)` signatures to signify 
> prototypeless C functions. I would like to see more details of how we plan to 
> handle this in the backend before I have any idea about what anyone else 
> might thing.
Actually, I just noticed that we don't appear to have made this actually 
target-dependent (the test uses X86). So we should either make this attribute 
actually wasm-only, or make it an enum.


Repository:
  rC Clang

https://reviews.llvm.org/D48443



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

[PATCH] D57155: [WebAssembly] Add a __wasi__ target macro

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

LGTM; don't forget to include all the context in the future


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57155



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

[PATCH] D59448: [WebAssembly] Change wasm.throw's first argument to an immediate

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

LGTM; I wonder if it makes sense to have predefined macro for the C++ tag (or 
perhaps just a regular macro for use in libcxxabi?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59448



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

[PATCH] D59743: [WebAssembly] Don't use default GetLinkerPath

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Reverted in rG039be787914610c28cba45c4557454e0a96939ab 
. Caused a 
strange error with the waterfall sysroot's build of libcxx: 
https://logs.chromium.org/logs/wasm/buildbucket/cr-buildbucket.appspot.com/8917800786005174656/+/steps/libcxx/0/stdout


Repository:
  rC Clang

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

https://reviews.llvm.org/D59743



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

[PATCH] D52856: [WebAssembly] __builtin_wasm_replace_lane_* builtins

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

W is int64_t (which is long long for wasm32 and would probably be long for 
wasm64 since that would probably LP64 (like Linux) rather than LLP64 (Like 
Win64). So if we want it to be the same for both wasm32 and wasm64, I guess we 
want LL.


Repository:
  rC Clang

https://reviews.llvm.org/D52856



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

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

So this CL has the effect that setting `-pthreads` will also set `-matomics`.
Currently as you mentioned we have the problem that we can't make our current 
logic of "do we lower away the atomics" be controlled by the target features 
because it's done at pass config time and not codegen time (where there's no 
access to the per-function subtarget). 
Also IIUC on ARM, it's `-mthread-model` that controls generation of atomic 
instructions, so it makes sense that `-mthread-model` does the same thing for 
Wasm. But it's also true that for wasm we use `-m` to enable codegen 
for a particular wasm feature, so it would be good to make `-matomics` do the 
same for consistency.
So, can we just pin those 2 flags together here? i.e. setting one will just 
cause the other to be set?
(I guess we'd also need consistency check so that if someone does something 
like `-mthread-model=posix -mno-atomics` we either throw an error or make it so 
that one of those always overrides the other).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Oh I guess another option would just be to pin all 3 flags together here, but 
since `-pthread` sets a preprocessor define and may also affect linker 
behavior, I think it's fine to allow atomic codegen without setting `-pthread`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

[PATCH] D63081: [WebAssembly] Cleanup toolchain test files. NFC.

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63081



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

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

If there's any chance this TLS ABI could be useful for WASI (I don't know if 
there's been any WASI work on threads yet, but it seems like there's no reason 
it couldn't be), then we should start a doc in tool-conventions for it. If not 
then we should get it behind the emscripten OS in LLVM. (and document it 
anyway; either in tool-conventions or somewhere in the emscripten site).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64537



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

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Oh, btw, any reason these have to be passive segments? Why can't we just make 
them active segments and let the VM initialize them for us?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64537



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

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

The `offset` field of a segment can be a constant expression 

 which can be a `global.get` of an imported global. So we could have an 
imported global `__tls_base` which is different for each thread, and have an 
active segment with that as its segment offset?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64537



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

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Another high-level question (based just on reading the CL description): The 
TLS-size intrinsic is per-function, does that mean that the tls-init function 
is called for every function? are there just multiple TLS sections per object 
file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64537



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

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

I had a reply that got eaten here, so I'm going to keep trolling you on your CL 
since we don't have a design doc for this.
The `offset` field of a data segment initializer can be a `global.get` on an 
imported global.  
(https://webassembly.github.io/spec/core/valid/instructions.html#constant-expressions).
 Since each thread is separately instantiated with separate JS, we could have a 
global import like `__tls_base` which has a different value in each thread. 
Then we wouldn't need to manually call the init code anywhere. Would there be 
other advantages or disadvantages for that?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64537



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

[PATCH] D66168: [WebAssembly] Make clang emit correct va_arg code for structs

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

LGTM. Nice that we had a test which tested the wrong thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66168



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

[PATCH] D66255: [WebAssembly] Correctly handle va_arg of zero-sized structures

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/test/CodeGen/wasm-varargs.c:104
+
+struct S test_zero_size_struct(char *fmt, ...) {
+  va_list va;

This should maybe be called "test_empty_struct" since its size is actually 1 
and not zero.



Comment at: clang/test/CodeGen/wasm-varargs.c:115
+
+// CHECK: define void @test_zero_size_struct([[STRUCT_S:%[^,=]+]]*{{.*}} 
noalias sret [[AGG_RESULT:%.*]], i8*{{.*}} %fmt, ...) {{.*}} {
+// CHECK:   [[FMT_ADDR:%[^,=]+]] = alloca i8*, align 4

could use CHECK_LABEL here and above.



Comment at: clang/test/CodeGen/wasm-varargs.c:125
+// CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
+// CHECK:   [[R0:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_Z]]*
+// CHECK:   [[R1:%[^,=]+]] = bitcast [[STRUCT_Z]]* [[U]] to i8*

It looks like most of the output (up to here) is boilerplate for va_arg setup 
and duplicates the CHECKs above. It might be more clear if we just omitted the 
duplicated stuff and only CHECK for the things that are different from the 
above test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66255



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

[PATCH] D69075: [WebAssembly] -pthread implies -target-feature +sign-ext

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/wasm-toolchain.c:76
 
+// '-pthread' not allowed with '-mno-mutable-globals'
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \

`-mno-sign-ext`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69075



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

[PATCH] D80362: [WebAssembly] Warn on exception spec only when Wasm EH is used

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

otherwise LGTM




Comment at: clang/docs/DiagnosticsReference.rst:14018
++---+
+|:warning:`warning:` |nbsp| :diagtext:`dynamic exception specifications with 
types are currently ignored in wasm exception handling`|
++---+

I think it's not actually necessary to change the text of the warning. Since 
the warning is about an exception handling language feature, I think adding 
"exception handling" on the end doesn't make it any more clear and sounds a 
little redundant to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80362



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

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Yeah I think a 64-bit version of userstack.ll makes sense.  (a fork or a second 
set of CHECKs, whatever you think would be cleaner). There's also 
`stack-alignment.ll` which covers dynamic stack adjustment.




Comment at: lld/wasm/Driver.cpp:385
+StringRef s = arg->getValue();
+if (s == "wasm32")
+  config->is64 = false;

any particular reason this shouldn't use the more conventional `-m32`/`m64`?
edit: nevermind, this is lld not clang. I'll defer to Sam's opinion on lld 
flags.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:84
 WasmSym->setGlobalType(wasm::WasmGlobalType{
-uint8_t(Subtarget.hasAddr64() ? wasm::WASM_TYPE_I64
-  : wasm::WASM_TYPE_I32),
+uint8_t(Subtarget.hasAddr64() && strcmp(Name, "__table_base") != 0
+? wasm::WASM_TYPE_I64

should __table_base stay as i32?


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

https://reviews.llvm.org/D82130



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

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

So the code LGTM, were you going to add to usertest.ll in this CL?




Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:84
 WasmSym->setGlobalType(wasm::WasmGlobalType{
-uint8_t(Subtarget.hasAddr64() ? wasm::WASM_TYPE_I64
-  : wasm::WASM_TYPE_I32),
+uint8_t(Subtarget.hasAddr64() && strcmp(Name, "__table_base") != 0
+? wasm::WASM_TYPE_I64

aardappel wrote:
> dschuff wrote:
> > should __table_base stay as i32?
> I'd think so, right? since it refers to table indices, not memory
Oh, right; I'd misread this as setting it just for table_base but I had it 
backwards; it's exempting table_base. So yeah this is right.


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

https://reviews.llvm.org/D82130



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

[PATCH] D82130: [WebAssembly] Adding 64-bit versions of __stack_pointer and other globals

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

Compiler changes LGTM




Comment at: llvm/test/CodeGen/WebAssembly/stack-alignment.ll:1
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt 
-wasm-keep-registers | FileCheck %s
-
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false 
-disable-wasm-fallthrough-return-opt -wasm-keep-registers | FileCheck -DPTR=32 
%s
+; RUN: llc < %s --mtriple=wasm64-unknown-unknown -asm-verbose=false 
-disable-wasm-fallthrough-return-opt -wasm-keep-registers | FileCheck -DPTR=64 
%s

that -D flag is really nice.


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

https://reviews.llvm.org/D82130



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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

@jfb thanks for the heads-up.
I replied on the mailing list thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

We can definitely still talk about what you are trying to do, and it would 
probably be useful to have more folks involved. Opening an issue on 
https://github.com/WebAssembly/tool-conventions/ might be useful since it 
involves the conventions that LLVM and clang use. If it's specific to the web, 
then https://groups.google.com/forum/#!forum/emscripten-discuss could be 
another place (even if you don't plan on using emscripten's JS code).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added reviewers: sbc100, aardappel.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, 
aprantl.
Herald added projects: clang, LLVM.
dschuff requested review of this revision.
Herald added a subscriber: aheejin.

Initial support for dwarf fission sections (-gsplit-dwarf) on wasm.
The most interesting change is support for writing 2 files (.o and .dwo) in the
wasm object writer. My approach moves object-writing logic into its own function
and calls it twice, swapping out the endian::Writer (W) in between calls.
(This is a bit different than the ELF writer's approach. I like it a bit better
but don't have a strong opinion).

This patch has the basic structure and writes separate files containing
non-dwo and dwo sections. There are couple of other things needed:

1. Linker support for an equivalent of ELF's SHF_EXCLUDE to keep the debug 
sections from being linked
2. a few checks and validations (equivalent to the places where the ELF object 
writer checks the dwo mode, such as validating relocs)

These can go in future CLs, but I still need to add some tests to this one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85685

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/lib/MC/MCAsmBackend.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp

Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -270,8 +270,8 @@
 DwoOnly,
   };
   bool IsSplitDwarf = false;
-  raw_pwrite_stream* OS = nullptr;
-  raw_pwrite_stream* DwoOS = nullptr;
+  raw_pwrite_stream *OS = nullptr;
+  raw_pwrite_stream *DwoOS = nullptr;
 
   // TargetObjectWriter wranppers.
   bool is64Bit() const { return TargetObjectWriter->is64Bit(); }
@@ -287,9 +287,8 @@
   : TargetObjectWriter(std::move(MOTW)), OS(&OS_) {}
   WasmObjectWriter(std::unique_ptr MOTW,
raw_pwrite_stream &OS_, raw_pwrite_stream &DwoOS_)
-  : TargetObjectWriter(std::move(MOTW)),
-IsSplitDwarf(true), OS(&OS_), DwoOS(&DwoOS_) {}
-
+  : TargetObjectWriter(std::move(MOTW)), IsSplitDwarf(true), OS(&OS_),
+DwoOS(&DwoOS_) {}
 
 private:
   void reset() override {
@@ -324,8 +323,8 @@
 
   uint64_t writeObject(MCAssembler &Asm, const MCAsmLayout &Layout) override;
 
-  uint64_t writeOneObject(MCAssembler &Asm, const MCAsmLayout &Layout, DwoMode Mode);
-
+  uint64_t writeOneObject(MCAssembler &Asm, const MCAsmLayout &Layout,
+  DwoMode Mode);
 
   void writeString(const StringRef Str) {
 encodeULEB128(Str.size(), W->OS);
@@ -781,7 +780,7 @@
   break;
 case wasm::WASM_EXTERNAL_MEMORY:
   encodeULEB128(Import.Memory.Flags, W->OS);
-  encodeULEB128(NumPages, W->OS);  // initial
+  encodeULEB128(NumPages, W->OS); // initial
   break;
 case wasm::WASM_EXTERNAL_TABLE:
   W->OS << char(Import.Table.ElemType);
@@ -961,8 +960,8 @@
   encodeULEB128(0, W->OS); // memory index
 if ((Segment.InitFlags & wasm::WASM_SEGMENT_IS_PASSIVE) == 0) {
   W->OS << char(Segment.Offset > std::numeric_limits().max()
- ? wasm::WASM_OPCODE_I64_CONST
- : wasm::WASM_OPCODE_I32_CONST);
+? wasm::WASM_OPCODE_I64_CONST
+: wasm::WASM_OPCODE_I32_CONST);
   encodeSLEB128(Segment.Offset, W->OS); // offset
   W->OS << char(wasm::WASM_OPCODE_END);
 }
@@ -1771,6 +1770,7 @@
 
 std::unique_ptr
 llvm::createWasmDwoObjectWriter(std::unique_ptr MOTW,
-raw_pwrite_stream &OS, raw_pwrite_stream &DwoOS) {
+raw_pwrite_stream &OS,
+raw_pwrite_stream &DwoOS) {
   return std::make_unique(std::move(MOTW), OS);
 }
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -796,8 +796,10 @@
   DwarfFrameSection = Ctx->getWasmSection(".debug_frame", SectionKind::getMetadata());
   DwarfPubNamesSection = Ctx->getWasmSection(".debug_pubnames", SectionKind::getMetadata());
   DwarfPubTypesSection = Ctx->getWasmSection(".debug_pubtypes", SectionKind::getMetadata());
-  DwarfGnuPubNamesSection = Ctx->getWasmSection(".debug_gnu_pubnames", SectionKind::getMetadata());
-  DwarfGnuPubTypesSection = Ctx->getWasmSection(".debug_gnu_pubtypes", SectionKind::getMetadata());
+  DwarfGnuPubNamesSection =
+  Ctx->getWasmSection(".debug_gnu_pubnames", SectionKind::getMetadata());
+  DwarfGnuPubTypesSection =
+  Ctx->getWasmSection(".debug_gnu_pubtypes", SectionKind::getMetadata());
 
   DwarfDebugNamesSection =
   Ctx->getWasmSection(".debug_names", SectionKind::getMetadata());
@@ -817,15 +819,15 @@
   Ctx->getWasmSection(".debug_types.dwo", SectionKind::getMetadata());
  

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 284522.
dschuff added a comment.

Fix bad diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/MC/MCWasmObjectWriter.h
  llvm/lib/MC/MCAsmBackend.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp

Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -216,8 +216,12 @@
   Stream.pwrite((char *)Buffer, sizeof(Buffer), Offset);
 }
 
+bool isDwoSection(const MCSection &Sec) {
+  return Sec.getName().endswith(".dwo");
+}
+
 class WasmObjectWriter : public MCObjectWriter {
-  support::endian::Writer W;
+  support::endian::Writer *W;
 
   /// The target specific Wasm writer instance.
   std::unique_ptr TargetObjectWriter;
@@ -260,7 +264,16 @@
   unsigned NumEventImports = 0;
   uint32_t SectionCount = 0;
 
-  // TargetObjectWriter wrappers.
+  enum class DwoMode {
+AllSections,
+NonDwoOnly,
+DwoOnly,
+  };
+  bool IsSplitDwarf = false;
+  raw_pwrite_stream *OS = nullptr;
+  raw_pwrite_stream *DwoOS = nullptr;
+
+  // TargetObjectWriter wranppers.
   bool is64Bit() const { return TargetObjectWriter->is64Bit(); }
   bool isEmscripten() const { return TargetObjectWriter->isEmscripten(); }
 
@@ -270,8 +283,12 @@
 
 public:
   WasmObjectWriter(std::unique_ptr MOTW,
-   raw_pwrite_stream &OS)
-  : W(OS, support::little), TargetObjectWriter(std::move(MOTW)) {}
+   raw_pwrite_stream &OS_)
+  : TargetObjectWriter(std::move(MOTW)), OS(&OS_) {}
+  WasmObjectWriter(std::unique_ptr MOTW,
+   raw_pwrite_stream &OS_, raw_pwrite_stream &DwoOS_)
+  : TargetObjectWriter(std::move(MOTW)), IsSplitDwarf(true), OS(&OS_),
+DwoOS(&DwoOS_) {}
 
 private:
   void reset() override {
@@ -306,24 +323,27 @@
 
   uint64_t writeObject(MCAssembler &Asm, const MCAsmLayout &Layout) override;
 
+  uint64_t writeOneObject(MCAssembler &Asm, const MCAsmLayout &Layout,
+  DwoMode Mode);
+
   void writeString(const StringRef Str) {
-encodeULEB128(Str.size(), W.OS);
-W.OS << Str;
+encodeULEB128(Str.size(), W->OS);
+W->OS << Str;
   }
 
   void writeI32(int32_t val) {
 char Buffer[4];
 support::endian::write32le(Buffer, val);
-W.OS.write(Buffer, sizeof(Buffer));
+W->OS.write(Buffer, sizeof(Buffer));
   }
 
   void writeI64(int64_t val) {
 char Buffer[8];
 support::endian::write64le(Buffer, val);
-W.OS.write(Buffer, sizeof(Buffer));
+W->OS.write(Buffer, sizeof(Buffer));
   }
 
-  void writeValueType(wasm::ValType Ty) { W.OS << static_cast(Ty); }
+  void writeValueType(wasm::ValType Ty) { W->OS << static_cast(Ty); }
 
   void writeTypeSection(ArrayRef Signatures);
   void writeImportSection(ArrayRef Imports, uint64_t DataSize,
@@ -368,17 +388,17 @@
 void WasmObjectWriter::startSection(SectionBookkeeping &Section,
 unsigned SectionId) {
   LLVM_DEBUG(dbgs() << "startSection " << SectionId << "\n");
-  W.OS << char(SectionId);
+  W->OS << char(SectionId);
 
-  Section.SizeOffset = W.OS.tell();
+  Section.SizeOffset = W->OS.tell();
 
   // The section size. We don't know the size yet, so reserve enough space
   // for any 32-bit value; we'll patch it later.
-  encodeULEB128(0, W.OS, 5);
+  encodeULEB128(0, W->OS, 5);
 
   // The position where the section starts, for measuring its size.
-  Section.ContentsOffset = W.OS.tell();
-  Section.PayloadOffset = W.OS.tell();
+  Section.ContentsOffset = W->OS.tell();
+  Section.PayloadOffset = W->OS.tell();
   Section.Index = SectionCount++;
 }
 
@@ -388,19 +408,19 @@
   startSection(Section, wasm::WASM_SEC_CUSTOM);
 
   // The position where the section header ends, for measuring its size.
-  Section.PayloadOffset = W.OS.tell();
+  Section.PayloadOffset = W->OS.tell();
 
   // Custom sections in wasm also have a string identifier.
   writeString(Name);
 
   // The position where the custom section starts.
-  Section.ContentsOffset = W.OS.tell();
+  Section.ContentsOffset = W->OS.tell();
 }
 
 // Now that the section is complete and we know how big it is, patch up the
 // section size field at the start of the section.
 void WasmObjectWriter::endSection(SectionBookkeeping &Section) {
-  uint64_t Size = W.OS.tell();
+  uint64_t Size = W->OS.tell();
   // /dev/null doesn't support seek/tell and can report offset of 0.
   // Simply skip this patching in that case.
   if (!Size)
@@ -414,14 +434,14 @@
 
   // Write the final section size to the payload_len field, which follows
   // the section id byte.
-  writePatchableLEB<5>(static_cast(W.OS), Size,
+  writePatchableLEB<5>(static_cast(W->OS), Size,
Section.SizeOffset);
 }
 
 // Emit the Wasm header.
 void WasmObjectWrit

[PATCH] D79655: [WebAssembly] Ignore exception specifications

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Actually, would it be possible to not ignore `throw()` but make it an alias for 
`noexcept(true)`? Apparently that is the standard behavior in C++17, so it 
might make more sense to just implement that now rather than just warning all 
the time and ignoring it. It would also cover most of the cases that exist, so 
more users wouldn't need to disable the warning.




Comment at: clang/lib/CodeGen/CGException.cpp:23
 #include "clang/AST/StmtVisitor.h"
+#include "clang/Basic/DiagnosticSema.h"
 #include "clang/Basic/TargetBuiltins.h"

aheejin wrote:
> aheejin wrote:
> > One thing I'm not sure about is if it is a good thing to include this in 
> > CodeGen. Usually this header is used in Sema/ directory. This depends on 
> > which .td file I add `warn_wasm_exception_spec_ignored`. 
> > DiagnosticSemaKinds.td seemed to have a section in exception spec so I 
> > added it there, but not entirely sure if that's the best location to add it.
> Oh, and this warning message was requested by Adobe, but I think it's good to 
> have in general.
According to the CMakeLists.txt in lib/CodeGen,  clangCodeGen depends on 
clangBasic, so I think it should be ok to include stuff from clang/Basic here 
(even if it has Sema in the name).
And a warning seems ok, as long as it's possible to suppress it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79655



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

[PATCH] D79655: [WebAssembly] Handle exception specifications

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/CodeGenCXX/wasm-eh.cpp:399
 
+// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions 
-fexceptions -fcxx-exceptions -fwasm-exceptions -target-feature 
+exception-handling -S -o - -std=c++11 | FileCheck %s --check-prefix=ASSEMBLY
+

aheejin wrote:
> This was preexisting just moved
Is it common in these tests to have RUN lines throughout the file instead of 
all together up at the top?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79655



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

[PATCH] D149917: [lld][WebAssembly] Add --keep-section flag

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lld/wasm/Options.td:196
 
+defm keep_section: Eq<"keep-section",
+ "Preserve a section even when --strip-all is given.  This is useful for 
compiler drivers such as clang or emcc that, for example, depend on the 
features section for post-link processing.">;

Can this flag be used more than once? From the code it looks like maybe it can, 
but the description should maybe say that and there should be a test (probably 
I should have written one for objcopy/strip too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D149917: [lld][WebAssembly] Add --keep-section flag

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

otherwise LGTM




Comment at: lld/wasm/Options.td:196
 
+defm keep_section: Eq<"keep-section",
+ "Preserve a section even when --strip-all is given.  This is useful for 
compiler drivers such as clang or emcc that, for example, depend on the 
features section for post-link processing.">;

dschuff wrote:
> Can this flag be used more than once? From the code it looks like maybe it 
> can, but the description should maybe say that and there should be a test 
> (probably I should have written one for objcopy/strip too)
Should this description also say that keep-section can be used more than once? 
or is there some other way to know that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149917

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

[PATCH] D118573: [clang][WebAssemmbly] Call TargetInfo::adjust in derived method.

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

Looks OK to me. All of these options handle explicit overrides to the ABI which 
I guess just didn't work before but doesn't change any defaults. We should 
probably put something in the emscripten release notes with a list of the flags 
that now actually do something, just in case someone was accidentally using 
them before :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118573

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

[PATCH] D123763: [randstruct] Enforce using a designated init for a randomized struct

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D123763#3485836 , @sbc100 wrote:

> This new test has been failing on the emscripten builders.. seemingly ever 
> since it landed:

This uses a fairly old sysroot. I tried with a new version of libcxx, and it 
seems to be passing. So maybe this is depending on some new C++ that our old 
sysroot doesn't support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123763

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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

I think the expectation is that if you use an intrinsics header that has an 
intrinsic for each machine instruction, that each intrinsic call should result 
in exactly one machine instruction with the same arguments you passed to it. If 
the vector operation is created some other way (e.g. autovectorization or even 
__builtin_shufflevector) then I agree that there doesn't necessarily have to be 
that expectation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Oh, interesting I didn't notice that those are implementations of the 
target-specific intrinsics. I wonder if they do that so they can implement the 
intrinsics on hardware that doesn't support the corresponding instruction? In a 
situation other than that I think I'd be surprised if I used a builtin for a 
particular instruction and got something else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

If this is to be like `-fdwarf-exceptions` I assume the idea is that eventually 
it will default on when targeting wasm, and become one of those flags that most 
users never set. In that case it really just selects the default exception 
model, and thus it should be compatible with `-fno-exceptions` just as 
`-fdwarf-exceptions`.
Also `-fno-exceptions` doesn't really mean "no exceptions whatsoever" because 
if you call something that the compiler isn't sure never throws, it generates 
an implicit catch-all that calls `std::terminate`. So how it does that would 
still be affected by the exception model. (and whatever downstream 
invoke-removing pass or postprocessing tool might care).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67208



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

[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Oh, you're right, I'm conflating `-mexception-handling` with `-fexceptions`. 
And also `-fno-exceptions` with `noexcept`. So... just forget I was here 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67208



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

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 292055.
dschuff added a comment.

Get the right sections in the objects, add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

Files:
  llvm/lib/MC/WasmObjectWriter.cpp


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -321,9 +321,8 @@
 
   void executePostLayoutBinding(MCAssembler &Asm,
 const MCAsmLayout &Layout) override;
-  void prepareImports(SmallVectorImpl& Imports,
-  MCAssembler &Asm,
-  const MCAsmLayout &Layout);
+  void prepareImports(SmallVectorImpl &Imports,
+  MCAssembler &Asm, const MCAsmLayout &Layout);
   uint64_t writeObject(MCAssembler &Asm, const MCAsmLayout &Layout) override;
 
   uint64_t writeOneObject(MCAssembler &Asm, const MCAsmLayout &Layout,
@@ -963,7 +962,7 @@
   encodeULEB128(0, W->OS); // memory index
 if ((Segment.InitFlags & wasm::WASM_SEGMENT_IS_PASSIVE) == 0) {
   W->OS << char(Segment.Offset > INT32_MAX ? wasm::WASM_OPCODE_I64_CONST
-  : wasm::WASM_OPCODE_I32_CONST);
+   : wasm::WASM_OPCODE_I32_CONST);
   encodeSLEB128(Segment.Offset, W->OS); // offset
   W->OS << char(wasm::WASM_OPCODE_END);
 }
@@ -1198,9 +1197,9 @@
 
   return true;
 }
-void WasmObjectWriter::prepareImports(SmallVectorImpl& 
Imports,
-  MCAssembler &Asm,
-  const MCAsmLayout &Layout) {
+void WasmObjectWriter::prepareImports(
+SmallVectorImpl &Imports, MCAssembler &Asm,
+const MCAsmLayout &Layout) {
   // For now, always emit the memory import, since loads and stores are not
   // valid without it. In the future, we could perhaps be more clever and omit
   // it if there are no loads or stores.


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -321,9 +321,8 @@
 
   void executePostLayoutBinding(MCAssembler &Asm,
 const MCAsmLayout &Layout) override;
-  void prepareImports(SmallVectorImpl& Imports,
-  MCAssembler &Asm,
-  const MCAsmLayout &Layout);
+  void prepareImports(SmallVectorImpl &Imports,
+  MCAssembler &Asm, const MCAsmLayout &Layout);
   uint64_t writeObject(MCAssembler &Asm, const MCAsmLayout &Layout) override;
 
   uint64_t writeOneObject(MCAssembler &Asm, const MCAsmLayout &Layout,
@@ -963,7 +962,7 @@
   encodeULEB128(0, W->OS); // memory index
 if ((Segment.InitFlags & wasm::WASM_SEGMENT_IS_PASSIVE) == 0) {
   W->OS << char(Segment.Offset > INT32_MAX ? wasm::WASM_OPCODE_I64_CONST
-  : wasm::WASM_OPCODE_I32_CONST);
+   : wasm::WASM_OPCODE_I32_CONST);
   encodeSLEB128(Segment.Offset, W->OS); // offset
   W->OS << char(wasm::WASM_OPCODE_END);
 }
@@ -1198,9 +1197,9 @@
 
   return true;
 }
-void WasmObjectWriter::prepareImports(SmallVectorImpl& Imports,
-  MCAssembler &Asm,
-  const MCAsmLayout &Layout) {
+void WasmObjectWriter::prepareImports(
+SmallVectorImpl &Imports, MCAssembler &Asm,
+const MCAsmLayout &Layout) {
   // For now, always emit the memory import, since loads and stores are not
   // valid without it. In the future, we could perhaps be more clever and omit
   // it if there are no loads or stores.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85685: [WIP] Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 292056.
dschuff added a comment.
Herald added subscribers: sstefan1, ormris, jgravelle-google.
Herald added a reviewer: jdoerfert.

upload the correct diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/split-debug.c
  llvm/include/llvm/MC/MCWasmObjectWriter.h
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/MC/MCAsmBackend.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/fission-cu.ll
  llvm/test/DebugInfo/WebAssembly/fission-sections.ll

Index: llvm/test/DebugInfo/WebAssembly/fission-sections.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/fission-sections.ll
@@ -0,0 +1,48 @@
+; RUN: llc -split-dwarf-file=baz.dwo -split-dwarf-output=%t.dwo  -O0 %s -mtriple=wasm32-unknown-unknown -filetype=obj -o %t
+; RUN: llvm-objdump -h %t | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-objdump -h %t.dwo | FileCheck --check-prefix=DWO %s
+
+
+; This test is derived from test/DebugInfo/X86/fission-cu.ll
+; But it checks that the output objects have the expected sections
+
+source_filename = "test/DebugInfo/WebAssembly/fission-cu.ll"
+
+@a = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "a", scope: null, file: !2, line: 1, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "baz.c", directory: "/usr/local/google/home/echristo/tmp")
+!3 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!4 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "baz.dwo", emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+!6 = !{!0}
+!7 = !{i32 1, !"Debug Info Version", i32 3}
+
+; CHECK-LABEL: Sections:
+
+; OBJ: Idx Name
+; OBJ-NEXT: 0 IMPORT
+; OBJ-NEXT: DATACOUNT
+; OBJ-NEXT: DATA
+; OBJ-NEXT: .debug_abbrev
+; OBJ-NEXT: .debug_info
+; OBJ-NEXT: .debug_str
+; OBJ-NEXT: .debug_addr
+; OBJ-NEXT: .debug_pubnames
+; OBJ-NEXT: .debug_pubtypes
+; OBJ-NEXT: .debug_line
+; OBJ-NEXT: linking
+
+
+; DWO: Idx Name
+; DWO-NOT: IMPORT
+; DWO-NOT: DATA
+; DWO: 0 .debug_str.dwo
+; DWO-NEXT: .debug_str_offsets.dwo
+; DWO-NEXT: .debug_info.dwo
+; DWO-NEXT: .debug_abbrev.dwo
+; DWO-NEXT: producers
Index: llvm/test/DebugInfo/WebAssembly/fission-cu.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/fission-cu.ll
@@ -0,0 +1,121 @@
+; RUN: llc -split-dwarf-file=baz.dwo  -O0 %s -mtriple=wasm32-unknown-unknown -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v -all %t | FileCheck %s
+; RUN: llvm-readobj --relocations %t | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-objdump -h %t | FileCheck --check-prefix=HDR %s
+
+; This test is derived from test/DebugInfo/X86/fission-cu.ll
+
+source_filename = "test/DebugInfo/WebAssembly/fission-cu.ll"
+
+@a = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "a", scope: null, file: !2, line: 1, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "baz.c", directory: "/usr/local/google/home/echristo/tmp")
+!3 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!4 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "baz.dwo", emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+; Check that the skeleton compile unit contains the proper attributes:
+; This DIE has the following attributes: DW_AT_comp_dir, DW_AT_stmt_list,
+; DW_AT_low_pc, DW_AT_high_pc, DW_AT_ranges, DW_AT_dwo_name, DW_AT_dwo_id,
+; DW_AT_ranges_base, DW_AT_addr_base.
+
+; CHECK: .debug_abbrev contents:
+; CHECK: Abbrev table for offset: 0x
+; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_no
+; CHECK: DW_AT_stmt_list DW_FORM_sec_offset
+; CHECK: DW_AT_comp_dir  DW_FORM_strp
+; CHECK: DW_AT_GNU_dwo_name  DW_FORM_strp
+; CHECK: DW_AT_GNU_dwo_idDW_FORM_data8
+
+; Check that we're using the right forms.
+; CHECK: .debug_abbrev.dwo contents:
+; CHECK: Abbrev table for offset: 0x
+; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_yes
+; CHECK: DW_AT_producer  DW_FORM_GNU_str_index
+; CHECK: DW_AT_language  DW_FORM_data2
+; CHECK: DW_AT_name  DW_FORM_GNU_str_index
+; CHECK: DW_AT_GNU_dwo_name  DW_FORM_GNU_str_index
+; CHECK-NOT: DW_AT_low_pc
+; CHECK-NOT: DW_AT_stmt_l

[PATCH] D85685: Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

OK, I think this is ready to review for real. Can you take another look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D85685#2275585 , @sbc100 wrote:

> Seems reasonable.   Do you think this way is cleaner than the way elf does 
> it?   Looks like ELF creates two different ELFWriter inside the 
> ELFDwoObjectWriter subclass right?

Yeah, ELF splits ELFWriter out from ELFObjectWriter, and then instantiates it 
twice. It's all because ELFObjectWriter has to derive from MCObjectWriter which 
was clearly not designed with this in mind. I found the class split to be a bit 
awkward, but I don't really have strong feelings about it either way.

> Are we going need to wasm-ld tests to followup or is this really independent 
> of the linker?

It should be independent of the linker because the dwo files don't get linked 
by the linker. They can be used independently (or combined by the `dwp` tool 
but AFAIK it's simpler than the linker). And the object files are just the same 
as usual from the linker's perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D85685#2277998 , @dschuff wrote:

> 



> Yeah, ELF splits ELFWriter out from ELFObjectWriter, and then instantiates it 
> twice. It's all because ELFObjectWriter has to derive from MCObjectWriter 
> which was clearly not designed with this in mind. I found the class split to 
> be a bit awkward, but I don't really have strong feelings about it either way.

I should add that what I do instead is just have one instance, and just reset 
the relevant state before calling writeOneObject again. So the structure is 
cleaner but the downside of that is that the state has to be manually reset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

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

[PATCH] D85685: Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 292624.
dschuff added a comment.

rebase, address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

Files:
  llvm/lib/MC/WasmObjectWriter.cpp


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1309,8 +1309,9 @@
 support::endian::Writer DwoWriter(*DwoOS, support::little);
 W = &DwoWriter;
 return TotalSize + writeOneObject(Asm, Layout, DwoMode::DwoOnly);
+  } else {
+return writeOneObject(Asm, Layout, DwoMode::AllSections);
   }
-  return writeOneObject(Asm, Layout, DwoMode::AllSections);
 }
 
 uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1309,8 +1309,9 @@
 support::endian::Writer DwoWriter(*DwoOS, support::little);
 W = &DwoWriter;
 return TotalSize + writeOneObject(Asm, Layout, DwoMode::DwoOnly);
+  } else {
+return writeOneObject(Asm, Layout, DwoMode::AllSections);
   }
-  return writeOneObject(Asm, Layout, DwoMode::AllSections);
 }
 
 uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85685: Support dwarf fission for wasm object files

[Derek Schuff via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ff28fa6a756: Support dwarf fission for wasm object files 
(authored by dschuff).

Changed prior to commit:
  https://reviews.llvm.org/D85685?vs=292624&id=292626#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85685

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/split-debug.c
  llvm/include/llvm/MC/MCWasmObjectWriter.h
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/MC/MCAsmBackend.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/fission-cu.ll
  llvm/test/DebugInfo/WebAssembly/fission-sections.ll

Index: llvm/test/DebugInfo/WebAssembly/fission-sections.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/fission-sections.ll
@@ -0,0 +1,48 @@
+; RUN: llc -split-dwarf-file=baz.dwo -split-dwarf-output=%t.dwo  -O0 %s -mtriple=wasm32-unknown-unknown -filetype=obj -o %t
+; RUN: llvm-objdump -h %t | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-objdump -h %t.dwo | FileCheck --check-prefix=DWO %s
+
+
+; This test is derived from test/DebugInfo/X86/fission-cu.ll
+; But it checks that the output objects have the expected sections
+
+source_filename = "test/DebugInfo/WebAssembly/fission-cu.ll"
+
+@a = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "a", scope: null, file: !2, line: 1, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "baz.c", directory: "/usr/local/google/home/echristo/tmp")
+!3 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!4 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "baz.dwo", emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+!6 = !{!0}
+!7 = !{i32 1, !"Debug Info Version", i32 3}
+
+; CHECK-LABEL: Sections:
+
+; OBJ: Idx Name
+; OBJ-NEXT: 0 IMPORT
+; OBJ-NEXT: DATACOUNT
+; OBJ-NEXT: DATA
+; OBJ-NEXT: .debug_abbrev
+; OBJ-NEXT: .debug_info
+; OBJ-NEXT: .debug_str
+; OBJ-NEXT: .debug_addr
+; OBJ-NEXT: .debug_pubnames
+; OBJ-NEXT: .debug_pubtypes
+; OBJ-NEXT: .debug_line
+; OBJ-NEXT: linking
+
+
+; DWO: Idx Name
+; DWO-NOT: IMPORT
+; DWO-NOT: DATA
+; DWO: 0 .debug_str.dwo
+; DWO-NEXT: .debug_str_offsets.dwo
+; DWO-NEXT: .debug_info.dwo
+; DWO-NEXT: .debug_abbrev.dwo
+; DWO-NEXT: producers
Index: llvm/test/DebugInfo/WebAssembly/fission-cu.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/fission-cu.ll
@@ -0,0 +1,121 @@
+; RUN: llc -split-dwarf-file=baz.dwo  -O0 %s -mtriple=wasm32-unknown-unknown -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v -all %t | FileCheck %s
+; RUN: llvm-readobj --relocations %t | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-objdump -h %t | FileCheck --check-prefix=HDR %s
+
+; This test is derived from test/DebugInfo/X86/fission-cu.ll
+
+source_filename = "test/DebugInfo/WebAssembly/fission-cu.ll"
+
+@a = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = !DIGlobalVariable(name: "a", scope: null, file: !2, line: 1, type: !3, isLocal: false, isDefinition: true)
+!2 = !DIFile(filename: "baz.c", directory: "/usr/local/google/home/echristo/tmp")
+!3 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!4 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "baz.dwo", emissionKind: FullDebug, enums: !5, retainedTypes: !5, globals: !6, imports: !5)
+!5 = !{}
+; Check that the skeleton compile unit contains the proper attributes:
+; This DIE has the following attributes: DW_AT_comp_dir, DW_AT_stmt_list,
+; DW_AT_low_pc, DW_AT_high_pc, DW_AT_ranges, DW_AT_dwo_name, DW_AT_dwo_id,
+; DW_AT_ranges_base, DW_AT_addr_base.
+
+; CHECK: .debug_abbrev contents:
+; CHECK: Abbrev table for offset: 0x
+; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_no
+; CHECK: DW_AT_stmt_list DW_FORM_sec_offset
+; CHECK: DW_AT_comp_dir  DW_FORM_strp
+; CHECK: DW_AT_GNU_dwo_name  DW_FORM_strp
+; CHECK: DW_AT_GNU_dwo_idDW_FORM_data8
+
+; Check that we're using the right forms.
+; CHECK: .debug_abbrev.dwo contents:
+; CHECK: Abbrev table for offset: 0x
+; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_yes
+; CHECK: DW_AT_producer  DW_FORM_GNU_str_index
+; CHECK: DW_AT_language  DW_FORM_data2
+; CHECK: DW_AT_name  DW_FORM

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added reviewers: aardappel, sbc100.
Herald added subscribers: llvm-commits, cfe-commits, ecnelises, sunfish, 
hiraditya, jgravelle-google.
Herald added projects: clang, LLVM.
dschuff requested review of this revision.
Herald added subscribers: ormris, aheejin.

Since Wasm comdat sections work similarly to ELF, we can use that mechanism
to eliminate duplicate dwarf type information in the same way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,116 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5
+; RUN: llc -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlo

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

I may add a couple more tests to this, but I did want to ask @sbc100 about 
this, since I'm not 100% sure at the uniqueID field is for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dschuff wrote:
> I may add a couple more tests to this, but I did want to ask @sbc100 about 
> this, since I'm not 100% sure at the uniqueID field is for.
also let me be more clear about the question here: what is `UniqueID` for, and 
is it bad that I'm just passing it a number that is totally not unique?



Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1353
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())

sbc100 wrote:
> Just checking in the case where you were hitting this error the SectionName 
> was duplicate but the `Begin` is uniquified ?
> 
> 
right. There's a second .debug_info section, so the name of `Begin` gets 
uniquified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dblaikie wrote:
> dschuff wrote:
> > dschuff wrote:
> > > I may add a couple more tests to this, but I did want to ask @sbc100 
> > > about this, since I'm not 100% sure at the uniqueID field is for.
> > also let me be more clear about the question here: what is `UniqueID` for, 
> > and is it bad that I'm just passing it a number that is totally not unique?
> For ELF, at least, I believe the unique ID is used to know which elements are 
> to be treated as part of the same deduplication set.
> 
> If Wasm support in lld does the same thing, then using the same number for 
> every type unit would mean the linked binary would end up with only one type 
> definition - even when the input has many varied/independent type 
> definitions. Likely not what's intended.
For wasm I had thought that was what the 3rd argument (Group) was for. So if 
that's what `UniqueID` is for, then I have the same question about Group :)



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:24-30
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)

dblaikie wrote:
> Given that S.plit DWARF type units don't require comdat support (the dwp tool 
> will be aware of the type units and parse their boundaries, read their type 
> hash from the TU Header, etc). /may/ be worth separating that 
> functionality/testing from the comdat support/testing - but maybe not all 
> that interesting to separate it into two separate patches. (& if you find the 
> test coverage works better in one test, that's OK - just a thought)
I copied this test more-or-less directly from X86 :)
Although I will say it's kind of nice to test all the header types in one go 
like this just because it's slightly annoying to construct the LLVM metadata 
for debuginfo tests, so it's nice to be able to share that as much as possible. 
Previously we just didn't have enough coverage for split-dwarf anyway.



Comment at: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll:47
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset

dblaikie wrote:
> aardappel wrote:
> > I guess this doesn't actually test that only one copy of these remain after 
> > linking? That's already tested in LLD?
> Certainly lld shouldn't be tested here, no (llvm tests can't depend on other 
> subprojects like lld) - but yeah, some kind of comdat deduplication tests 
> should exist in lld (they don't have to be DWARF specific - the DWARF type 
> deduplication is meant to piggy-back on the existing comdat deduplication 
> infrastructure in thel inker)
@aardappel yes, pretty much what @dblaikie said :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:962
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   ~0);

dblaikie wrote:
> dschuff wrote:
> > dblaikie wrote:
> > > dschuff wrote:
> > > > dschuff wrote:
> > > > > I may add a couple more tests to this, but I did want to ask @sbc100 
> > > > > about this, since I'm not 100% sure at the uniqueID field is for.
> > > > also let me be more clear about the question here: what is `UniqueID` 
> > > > for, and is it bad that I'm just passing it a number that is totally 
> > > > not unique?
> > > For ELF, at least, I believe the unique ID is used to know which elements 
> > > are to be treated as part of the same deduplication set.
> > > 
> > > If Wasm support in lld does the same thing, then using the same number 
> > > for every type unit would mean the linked binary would end up with only 
> > > one type definition - even when the input has many varied/independent 
> > > type definitions. Likely not what's intended.
> > For wasm I had thought that was what the 3rd argument (Group) was for. So 
> > if that's what `UniqueID` is for, then I have the same question about Group 
> > :)
> Oh, fair enough - I hadn't read closely. Yeah, guess it's up to you folks/how 
> the wasm object format works... - so I'm with you on the "what is the 
> uniqueID field for" (& what's the other field that's taking the hash?) & I'll 
> leave it to you folks to... hash out.
@sbc100 ping, is this what we want here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301133.
dschuff added a comment.

use GenericSectionID instead of ~0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  llvm/lib/MC/MCObjectFileInfo.cpp


Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -965,7 +965,7 @@
   utostr(Hash));
   case Triple::Wasm:
 return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
-   ~0);
+   MCContext::GenericSectionID);
   case Triple::MachO:
   case Triple::COFF:
   case Triple::GOFF:


Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -965,7 +965,7 @@
   utostr(Hash));
   case Triple::Wasm:
 return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
-   ~0);
+   MCContext::GenericSectionID);
   case Triple::MachO:
   case Triple::COFF:
   case Triple::GOFF:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbcb8a119df21: [WebAssembly] Add support for DWARF type units 
(authored by dschuff).

Changed prior to commit:
  https://reviews.llvm.org/D88603?vs=301133&id=301136#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,116 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5
+; RUN: llc -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=5 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple= < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

This broke the bots for some strange reason that didn't reproduce locally. But 
because it was ~all of them, I probably just did something stupid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D88603#2359554 , @dblaikie wrote:

> In D88603#2357973 , @dschuff wrote:
>
>> This broke the bots for some strange reason that didn't reproduce locally. 
>> But because it was ~all of them, I probably just did something stupid.
>
> Good to have links to buildbots and/or quotes from buildbots (incase the logs 
> get retired) about the specifics so other folks can chip in/know what went 
> wrong, etc.

Example link is http://lab.llvm.org:8011/#/builders/109/builds/1402
But, as I suspected, I just did something stupid (namely, I thought I had 
assertions enabled, but I didn't)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301486.
dschuff added a comment.

use getOrCreate


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1378,9 +1378,6 @@
   MCSymbol *Begin = Sec.getBeginSymbol();
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())
-  report_fatal_error("section name and begin symbol should match: " +
- Twine(SectionName));
   }
 
   // Separate out the producers and target features sections
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -963,9 +963,11 @@
   case Triple::ELF:
 return Ctx->getELFSection(Name, ELF::SHT_PROGBITS, ELF::SHF_GROUP, 0,
   utostr(Hash));
+  case Triple::Wasm:
+return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
+   MCContext::GenericSectionID);
   case Triple::MachO:
   case Triple::COFF:
-  case Triple::Wasm:
   case Triple::GOFF:
   case Triple::XCOFF:
   case Triple::UnknownObjectFormat:
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -644,7 +644,7 @@
 
   StringRef CachedName = Entry.first.SectionName;
 
-  MCSymbol *Begin = createSymbol(CachedName, false, false);
+  MCSymbol *Begin = getOrCreateSymbol(CachedName);
   cast(Begin)->setType(wasm::WASM_SYMBOL_TYPE_SECTION);
 
   MCSectionWasm *Result = new (WasmAllocator.Allocate())
Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -394,8 +394,9 @@
 UseSectionsAsReferences = DwarfSectionsAsReferences == Enable;
 
   // Don't generate type units for unsupported object file formats.
-  GenerateTypeUnits =
-  A->TM.getTargetTriple().isOSBinFormatELF() && GenerateDwarfTypeUnits;
+  GenerateTypeUnits = (A->TM.getTargetTriple().isOSBinFormatELF() ||
+   A->TM.getTargetTriple().isOSBinFormatWasm()) &&
+  GenerateDwarfTypeUnits;
 
   TheAccelTableKind = computeAccelTableKind(
   DwarfVersion, GenerateTypeUnits, DebuggerTuning, 
A->TM.getTargetTriple());
Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -214,6 +214,9 @@
 // RUN: %clang -### -fdebug-types-section -fno-debug-types-section -target 
x86_64-unknown-linux %s 2>&1 \
 // RUN:| FileCheck -check-prefix=NOFDTS %s
 //
+// RUN: %clang -### -fdebug-types-section -target wasm32-unknown-unknown %s 
2>&1 \
+// RUN:| FileCheck -check-prefix=FDTS %s
+//
 // RUN: %clang -### -fdebug-types-section -target x86_64-apple-darwin %s 2>&1 \
 // RUN:| FileCheck -check-prefix=FDTSE %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3989,7 +3989,7 @@
 
   if (Args.hasFlag(options::OPT_fdebug_types_section,
options::OPT_fno_debug_types_section, false)) {
-if (!T.isOSBinFormatELF()) {
+if (!(T.isOSBinFormatELF() || T.isOSBinFormatWasm())) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << Args.getLastArg(options::OPT_fdebug_types_section)
  ->getAsString(Args)


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -1378,9 +1378,6 @@
   MCSymbol *Begin = Sec.getBeginSymbol();
   if (Begin) {
 WasmIndices[cast(Begin)] = CustomSections.size();
-if (SectionName != Begin->getName())
-  report_fatal_error("section name and begin symbol should match: " +
- Twine(SectionName));
   }
 
   // Separate out the producers and target features sections
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -963,9 +963,11 @@
   case Triple:

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301488.
dschuff added a comment.

can't just getOrCreate symbol without dealing with the section
just disable the test for now as we don't need dw5


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,117 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5 with type units
+; (See the FIXME in MCObjectFileInfo::getDwarfComdatSection)
+; RU N: llc -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RU N: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RU N: -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RU N: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "s", scope: !2, file: !3, line: 5, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_p

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 301490.
dschuff added a comment.

fix diff; it should be against master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

Index: llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll
@@ -0,0 +1,117 @@
+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
+
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN: -dwarf-version=4 -generate-type-units \
+; RUN: -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
+
+; TODO: enable testing for dwarf v5 with type units
+; (See the FIXME in MCObjectFileInfo::getDwarfComdatSection)
+; RU N: llc -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+
+; RU N: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RU N: -dwarf-version=5 -generate-type-units \
+; RU N: -filetype=obj -O0 -mtriple= < %s \
+; RU N: | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RU N: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+
+; This test is derived from test/CodeGen/X86/dwarf-headers.ll
+
+; Looking for DWARF headers to be generated correctly.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x0018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x0014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x0018: DW_TAG_type_unit
+; DWO-5: 0x0033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x0047: DW_TAG_compile_unit
+
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "s", scope: !2, file: !3, line: 5, type: !6, isLocal: false, isDefinition: true)
+!2 

[PATCH] D88603: [WebAssembly] Add support for DWARF type units

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

@sbc100 I found that the cause of the assertion is that 
in dwarf 5, the type units apparently go in the .debug_info section (instead of 
the .debug_type section), and this section already exists (but it was created 
as a non-comdat).
So when `getWasmSection` tries to look up an existing section it fails because 
the group is part of the key. Then it tries to create a new section but that 
fails because the name is duplicate.

For some reason this doesn't happen or is not a problem with ELF but I haven't 
looked up why yet.
For now I just disabled the dwarf5+TU tests since we don't really use dwarf5 
yet anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88603

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

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Can you upload the diff with full context (e.g. use `diff -U ` or use 
arcanist to upload)?

I'm a bit confused; the commit message talks about X32 being a separate 
architecture, but you're not adding any new architecture triples here (it still 
uses x86_64 as the architecture and selects the ABI via the environment). 
AFAICS what really changes is that the structure of the include and lib 
directories is now multi-arch style with the x32 headers alongside the x86_64 
base headers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

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

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

One other question then: do you know if Debian and/or Ubuntu still have the 
same support for running x32 programs on the regular x86-64 distribution? 
(presumably yes, since you aren't changing the existing behavior).
AFAIK clang's current support was developed against Ubuntu, but I haven't tried 
it in a long time and to my knowledge nobody has submitted any patches for x32 
in a while either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

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

[PATCH] D102791: [WebAssembly] Warn on exception spec for Emscripten EH

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

BTW Is there a way to disable this warning?
Since IIUC this could cause code that was not warning (because it used 
-fno-exceptions or used emscripten's default of -fignore-exceptions) to now 
start warning, sometimes that makes users who use -Werror unhappy.




Comment at: clang/lib/CodeGen/CGException.cpp:495
+CGM.getLangOpts().getExceptionHandling() ==
+LangOptions::ExceptionHandlingKind::None &&
+EST == EST_Dynamic)

Does emscripten's default of `-fignore-exceptions` also end up as haveing 
`ExceptionHandlingKind::None` even though exceptions aren't disabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102791

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

[PATCH] D102791: [WebAssembly] Warn on exception spec for Emscripten EH

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

Got it, this makes sense to me too. And since it can be turned off, I'm not too 
worried about annoying users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102791

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

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

It looks like this error is intended to catch mismatches for attributes that 
can affect codegen such as noreturn (in which case it makes sense to have it as 
an error) but it also now fires for cases such as `__attribute__(warning())` 
which often do not duplicate the attribute on the definition. Is that intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107613

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

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff created this revision.
dschuff added a reviewer: azakai.
Herald added subscribers: sunfish, hiraditya, jgravelle-google, sbc100.
dschuff requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added projects: clang, LLVM.

This fix goes along with d1a96e906cc03a95cfd41a1f22bdda92651250c7 

and makes the fp128 alignment match clang's long double alignment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105749

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/varargs.ll

Index: llvm/test/CodeGen/WebAssembly/varargs.ll
===
--- llvm/test/CodeGen/WebAssembly/varargs.ll
+++ llvm/test/CodeGen/WebAssembly/varargs.ll
@@ -2,8 +2,7 @@
 
 ; Test varargs constructs.
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 
 ; Test va_start.
 
@@ -167,21 +166,19 @@
 ; within a vararg buffer.
 
 ; CHECK-LABEL: call_fp128_alignment:
-; CHECK:  global.get  $push7=, __stack_pointer
-; CHECK-NEXT: i32.const   $push8=, 32
-; CHECK-NEXT: i32.sub $push12=, $pop7, $pop8
-; CHECK-NEXT: local.tee   $push11=, $1=, $pop12
-; CHECK-NEXT: global.set  __stack_pointer, $pop11
-; CHECK-NEXT: i32.const   $push0=, 24
+; CHECK:  global.get  $push5=, __stack_pointer
+; CHECK-NEXT: i32.const   $push6=, 32
+; CHECK-NEXT: i32.sub $push10=, $pop5, $pop6
+; CHECK-NEXT: local.tee   $push9=, $1=, $pop10
+; CHECK-NEXT: global.set  __stack_pointer, $pop9
+; CHECK-NEXT: i32.const   $push0=, 16
 ; CHECK-NEXT: i32.add $push1=, $1, $pop0
 ; CHECK-NEXT: i64.const   $push2=, -9223372036854775808
 ; CHECK-NEXT: i64.store   0($pop1), $pop2
-; CHECK-NEXT: i32.const   $push3=, 16
-; CHECK-NEXT: i32.add $push4=, $1, $pop3
-; CHECK-NEXT: i64.const   $push5=, 1
-; CHECK-NEXT: i64.store   0($pop4), $pop5
-; CHECK-NEXT: i32.const   $push6=, 7
-; CHECK-NEXT: i32.store   0($1), $pop6
+; CHECK-NEXT: i64.const   $push3=, 1
+; CHECK-NEXT: i64.store   8($1), $pop3
+; CHECK-NEXT: i32.const   $push4=, 7
+; CHECK-NEXT: i32.store   0($1), $pop4
 ; CHECK-NEXT: callcallee, $1
 define void @call_fp128_alignment(i8* %p) {
 entry:
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,12 +120,17 @@
 const Target &T, const Triple &TT, StringRef CPU, StringRef FS,
 const TargetOptions &Options, Optional RM,
 Optional CM, CodeGenOpt::Level OL, bool JIT)
-: LLVMTargetMachine(T,
-TT.isArch64Bit()
-? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1"
-: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1",
-TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
-getEffectiveCodeModel(CM, CodeModel::Large), OL),
+: LLVMTargetMachine(
+  T,
+  TT.isArch64Bit()
+  ? (TT.isOSEmscripten()
+ ? "e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1")
+  : (TT.isOSEmscripten()
+ ? "e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1"),
+  TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
+  getEffectiveCodeModel(CM, CodeModel::Large), OL),
   TLOF(new WebAssemblyTargetObjectFile()) {
   // WebAssembly type-checks instructions, but a noreturn function with a return
   // type that doesn't match the context will cause a check failure. So we lower
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -149,7 +149,10 @@
   explicit WebAssembly32TargetInfo(const llvm::Triple &T,
const TargetOptions &Opts)
   : WebAssemblyTargetInfo(T, Opts) {
-resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
@@ -168,7 +171,10 @@
 SizeType = UnsignedLong;
 PtrDiffType = SignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+ 

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a reviewer: sbc100.
dschuff added a comment.

This fix should go in ASAP (or else we should revert 
d1a96e906cc03a95cfd41a1f22bdda92651250c7 
) to fix 
the mismatch between LLVM and clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

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

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff updated this revision to Diff 357665.
dschuff added a comment.

- fix wasm64 copypasta


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/varargs.ll

Index: llvm/test/CodeGen/WebAssembly/varargs.ll
===
--- llvm/test/CodeGen/WebAssembly/varargs.ll
+++ llvm/test/CodeGen/WebAssembly/varargs.ll
@@ -2,8 +2,7 @@
 
 ; Test varargs constructs.
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 
 ; Test va_start.
 
@@ -167,21 +166,19 @@
 ; within a vararg buffer.
 
 ; CHECK-LABEL: call_fp128_alignment:
-; CHECK:  global.get  $push7=, __stack_pointer
-; CHECK-NEXT: i32.const   $push8=, 32
-; CHECK-NEXT: i32.sub $push12=, $pop7, $pop8
-; CHECK-NEXT: local.tee   $push11=, $1=, $pop12
-; CHECK-NEXT: global.set  __stack_pointer, $pop11
-; CHECK-NEXT: i32.const   $push0=, 24
+; CHECK:  global.get  $push5=, __stack_pointer
+; CHECK-NEXT: i32.const   $push6=, 32
+; CHECK-NEXT: i32.sub $push10=, $pop5, $pop6
+; CHECK-NEXT: local.tee   $push9=, $1=, $pop10
+; CHECK-NEXT: global.set  __stack_pointer, $pop9
+; CHECK-NEXT: i32.const   $push0=, 16
 ; CHECK-NEXT: i32.add $push1=, $1, $pop0
 ; CHECK-NEXT: i64.const   $push2=, -9223372036854775808
 ; CHECK-NEXT: i64.store   0($pop1), $pop2
-; CHECK-NEXT: i32.const   $push3=, 16
-; CHECK-NEXT: i32.add $push4=, $1, $pop3
-; CHECK-NEXT: i64.const   $push5=, 1
-; CHECK-NEXT: i64.store   0($pop4), $pop5
-; CHECK-NEXT: i32.const   $push6=, 7
-; CHECK-NEXT: i32.store   0($1), $pop6
+; CHECK-NEXT: i64.const   $push3=, 1
+; CHECK-NEXT: i64.store   8($1), $pop3
+; CHECK-NEXT: i32.const   $push4=, 7
+; CHECK-NEXT: i32.store   0($1), $pop4
 ; CHECK-NEXT: callcallee, $1
 define void @call_fp128_alignment(i8* %p) {
 entry:
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,12 +120,17 @@
 const Target &T, const Triple &TT, StringRef CPU, StringRef FS,
 const TargetOptions &Options, Optional RM,
 Optional CM, CodeGenOpt::Level OL, bool JIT)
-: LLVMTargetMachine(T,
-TT.isArch64Bit()
-? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1"
-: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1",
-TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
-getEffectiveCodeModel(CM, CodeModel::Large), OL),
+: LLVMTargetMachine(
+  T,
+  TT.isArch64Bit()
+  ? (TT.isOSEmscripten()
+ ? "e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1")
+  : (TT.isOSEmscripten()
+ ? "e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1"),
+  TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
+  getEffectiveCodeModel(CM, CodeModel::Large), OL),
   TLOF(new WebAssemblyTargetObjectFile()) {
   // WebAssembly type-checks instructions, but a noreturn function with a return
   // type that doesn't match the context will cause a check failure. So we lower
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -149,7 +149,10 @@
   explicit WebAssembly32TargetInfo(const llvm::Triple &T,
const TargetOptions &Opts)
   : WebAssemblyTargetInfo(T, Opts) {
-resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
@@ -168,7 +171,10 @@
 SizeType = UnsignedLong;
 PtrDiffType = SignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Basic/Targets/WebAssembly.h:175
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else

kripken wrote:
> Should this not be
> 
> resetDataLayout("e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1");
> 
> (that is, the first two numbers should be 64?)
oops yes, that's a copypasta


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

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

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGac02baab48c2: WebAssembly: Update datalayout to match fp128 
ABI change (authored by dschuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/varargs.ll

Index: llvm/test/CodeGen/WebAssembly/varargs.ll
===
--- llvm/test/CodeGen/WebAssembly/varargs.ll
+++ llvm/test/CodeGen/WebAssembly/varargs.ll
@@ -2,8 +2,7 @@
 
 ; Test varargs constructs.
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 
 ; Test va_start.
 
@@ -167,21 +166,19 @@
 ; within a vararg buffer.
 
 ; CHECK-LABEL: call_fp128_alignment:
-; CHECK:  global.get  $push7=, __stack_pointer
-; CHECK-NEXT: i32.const   $push8=, 32
-; CHECK-NEXT: i32.sub $push12=, $pop7, $pop8
-; CHECK-NEXT: local.tee   $push11=, $1=, $pop12
-; CHECK-NEXT: global.set  __stack_pointer, $pop11
-; CHECK-NEXT: i32.const   $push0=, 24
+; CHECK:  global.get  $push5=, __stack_pointer
+; CHECK-NEXT: i32.const   $push6=, 32
+; CHECK-NEXT: i32.sub $push10=, $pop5, $pop6
+; CHECK-NEXT: local.tee   $push9=, $1=, $pop10
+; CHECK-NEXT: global.set  __stack_pointer, $pop9
+; CHECK-NEXT: i32.const   $push0=, 16
 ; CHECK-NEXT: i32.add $push1=, $1, $pop0
 ; CHECK-NEXT: i64.const   $push2=, -9223372036854775808
 ; CHECK-NEXT: i64.store   0($pop1), $pop2
-; CHECK-NEXT: i32.const   $push3=, 16
-; CHECK-NEXT: i32.add $push4=, $1, $pop3
-; CHECK-NEXT: i64.const   $push5=, 1
-; CHECK-NEXT: i64.store   0($pop4), $pop5
-; CHECK-NEXT: i32.const   $push6=, 7
-; CHECK-NEXT: i32.store   0($1), $pop6
+; CHECK-NEXT: i64.const   $push3=, 1
+; CHECK-NEXT: i64.store   8($1), $pop3
+; CHECK-NEXT: i32.const   $push4=, 7
+; CHECK-NEXT: i32.store   0($1), $pop4
 ; CHECK-NEXT: callcallee, $1
 define void @call_fp128_alignment(i8* %p) {
 entry:
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,12 +120,17 @@
 const Target &T, const Triple &TT, StringRef CPU, StringRef FS,
 const TargetOptions &Options, Optional RM,
 Optional CM, CodeGenOpt::Level OL, bool JIT)
-: LLVMTargetMachine(T,
-TT.isArch64Bit()
-? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1"
-: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1",
-TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
-getEffectiveCodeModel(CM, CodeModel::Large), OL),
+: LLVMTargetMachine(
+  T,
+  TT.isArch64Bit()
+  ? (TT.isOSEmscripten()
+ ? "e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1")
+  : (TT.isOSEmscripten()
+ ? "e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1"
+ : "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1"),
+  TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
+  getEffectiveCodeModel(CM, CodeModel::Large), OL),
   TLOF(new WebAssemblyTargetObjectFile()) {
   // WebAssembly type-checks instructions, but a noreturn function with a return
   // type that doesn't match the context will cause a check failure. So we lower
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -149,7 +149,10 @@
   explicit WebAssembly32TargetInfo(const llvm::Triple &T,
const TargetOptions &Opts)
   : WebAssemblyTargetInfo(T, Opts) {
-resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:32:32-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
@@ -168,7 +171,10 @@
 SizeType = UnsignedLong;
 PtrDiffType = SignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
+if (T.isOSEmscripten())
+  resetDataLayout("e-m:e-p:64:64-i64:64-f128:64-n32:64-S128-ni:1");
+else
+  resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128-ni:1");
   }
 
 protected:
___
cfe-commits mailing list
cfe-

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/test/CodeGen/WebAssembly/varargs.ll:5
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 

sunfish wrote:
> It appears this change means that we're no longer testing varargs on 
> wasm32-unknown-unknown. Please update this test so that it tests both triples.
Done in rGd4e2693a679927a62dd738dd3bba24863dcd290a


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

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

[PATCH] D97834: [WebAssembly] Disable uses of __clang_call_terminate

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.

I agree this is a good approach, and I also like that it's smaller and simpler. 
In the future we can revisit whether following this particular Itanium 
convention buys us anything useful or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97834

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

[PATCH] D98457: [WebAssembly] Remove unimplemented-simd target features

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/docs/ClangCommandLineReference.rst:3801
 Pass -z  to the linker
-

extraneous change?



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:191
 
-TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", 
"unimplemented-simd128")
-TARGET_BUILTIN(__builtin_wasm_qfms_f32x4, "V4fV4fV4fV4f", "nc", 
"unimplemented-simd128")
-TARGET_BUILTIN(__builtin_wasm_qfma_f64x2, "V2dV2dV2dV2d", "nc", 
"unimplemented-simd128")
-TARGET_BUILTIN(__builtin_wasm_qfms_f64x2, "V2dV2dV2dV2d", "nc", 
"unimplemented-simd128")
+TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_qfms_f32x4, "V4fV4fV4fV4f", "nc", "simd128")

is QFMA actually in MVP simd? I thought it was non-deterministic?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1712
 };
-  } else if (NumConstantLanes >= NumSplatLanes &&
- Subtarget->hasUnimplementedSIMD128()) {
-// If we support v128.const, emit it directly
+  } else if (NumConstantLanes >= NumSplatLanes) {
 SmallVector ConstLanes;

out of curiosity, are there any cases where a splat could be smaller than a 
v128 const, since the consts are pretty big?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98457

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

[PATCH] D98466: [WebAssembly] Remove experimental SIMD instructions

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:191
 
-TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_qfms_f32x4, "V4fV4fV4fV4f", "nc", "simd128")

I guess this answers my question from the previous review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98466

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

[PATCH] D131217: [clang][WebAssembly] Pass `-Wl,-no-type-check` through to the MC layer

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

This looks good to me from the wasm perspective


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131217

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

This broke us in emscripten as well (building with trunk clang against a 
recent-but-not-trunk version of libcxx). I can test the fix if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Sorry, no :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441

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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Deprecation is progressing 
(https://groups.google.com/a/chromium.org/g/chromium-extensions/c/v8H1UHnPotY/m/NmzrIv_VBAAJ)
 but we are still supporting it on some platforms, (and using clang's upstream 
support), so we aren't there yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441

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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

In D78441#4602312 , @brad wrote:

> In D78441#4593287 , @dschuff wrote:
>
>> Deprecation is progressing 
>> (https://groups.google.com/a/chromium.org/g/chromium-extensions/c/v8H1UHnPotY/m/NmzrIv_VBAAJ)
>>  but we are still supporting it on some platforms, (and using clang's 
>> upstream support), so we aren't there yet.
>
> Ok, it's a bit confusing with different dates in different spots. So 
> according to that the mainstream OS (Win/macOS/Linux) releases of Chromium 
> will drop support by Dec 2023 just leaving ChromeOS support.

Correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441

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

[PATCH] D150803: [WebAssembly] Support `annotate` clang attributes for marking functions.

[Derek Schuff via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG220fe00a7c0f: [WebAssembly] Support `annotate` clang 
attributes for marking functions. (authored by brendandahl, committed by 
dschuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

Files:
  lld/test/wasm/func-attr-tombstone.s
  lld/test/wasm/func-attr.s
  lld/test/wasm/merge-func-attr-section.s
  lld/wasm/InputChunks.cpp
  lld/wasm/InputFiles.cpp
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/BinaryFormat/WasmRelocs.def
  llvm/include/llvm/MC/MCExpr.h
  llvm/lib/MC/MCExpr.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Object/WasmObjectFile.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyWasmObjectWriter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
  llvm/test/CodeGen/WebAssembly/func-attr-annotate.ll
  llvm/test/MC/WebAssembly/func-attr.s

Index: llvm/test/MC/WebAssembly/func-attr.s
===
--- /dev/null
+++ llvm/test/MC/WebAssembly/func-attr.s
@@ -0,0 +1,21 @@
+# RUN: llvm-mc -triple=wasm32-unknown-unknown < %s | FileCheck %s
+# Check that it also comiled to object for format.
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -o - < %s | obj2yaml | FileCheck -check-prefix=CHECK-OBJ %s
+
+foo:
+.globl foo
+.functype foo () -> ()
+end_function
+
+.section.custom_section.llvm.func_attr.custom0,"",@
+.int32  foo@FUNCINDEX
+
+# CHECK:   .section .custom_section.llvm.func_attr.custom0,"",@
+# CHECK-NEXT: .int32  foo@FUNCINDEX
+
+# CHECK-OBJ:- Type:CUSTOM
+# CHECK-OBJ-NEXT: Relocations:
+# CHECK-OBJ-NEXT:- Type:R_WASM_FUNCTION_INDEX_I32
+# CHECK-OBJ-NEXT:  Index:   0
+# CHECK-OBJ-NEXT:  Offset:  0x0
+# CHECK-OBJ-NEXT: Name:llvm.func_attr.custom0
Index: llvm/test/CodeGen/WebAssembly/func-attr-annotate.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/func-attr-annotate.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s -asm-verbose=false -wasm-keep-registers | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+@.str = private unnamed_addr constant [8 x i8] c"custom0\00", section "llvm.metadata"
+@.str.1 = private unnamed_addr constant [7 x i8] c"main.c\00", section "llvm.metadata"
+@.str.2 = private unnamed_addr constant [8 x i8] c"custom1\00", section "llvm.metadata"
+@.str.3 = private unnamed_addr constant [8 x i8] c"custom2\00", section "llvm.metadata"
+@llvm.global.annotations = appending global [3 x { ptr, ptr, ptr, i32, ptr }] [{ ptr, ptr, ptr, i32, ptr } { ptr @test0, ptr @.str, ptr @.str.1, i32 4, ptr null }, { ptr, ptr, ptr, i32, ptr } { ptr @test1, ptr @.str, ptr @.str.1, i32 5, ptr null }, { ptr, ptr, ptr, i32, ptr } { ptr @test2, ptr @.str.2, ptr @.str.1, i32 6, ptr null }], section "llvm.metadata"
+
+define void @test0() {
+  ret void
+}
+
+define void @test1() {
+  ret void
+}
+
+define void @test2() {
+  ret void
+}
+
+define void @test3() {
+  ret void
+}
+
+; CHECK:  .section.custom_section.llvm.func_attr.annotate.custom0,"",@
+; CHECK-NEXT: .int32  test0@FUNCINDEX
+; CHECK-NEXT: .int32  test1@FUNCINDEX
+; CHECK:  .section.custom_section.llvm.func_attr.annotate.custom1,"",@
+; CHECK-NEXT: .int32  test2@FUNCINDEX
Index: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
===
--- llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
+++ llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
@@ -66,6 +66,7 @@
   void emitEndOfAsmFile(Module &M) override;
   void EmitProducerInfo(Module &M);
   void EmitTargetFeatures(Module &M);
+  void EmitFunctionAttributes(Module &M);
   void emitSymbolType(const MCSymbolWasm *Sym);
   void emitGlobalVariable(const GlobalVariable *GV) override;
   void emitJumpTableInfo() override;
Index: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -27,6 +27,8 @@
 #include "WebAssemblyTargetMachine.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Analysis/ValueTracking.h"
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/AsmPrinter.h"
@@ -438,6 +440,7 @@
 
   EmitProducerInfo(M);
   EmitTargetFeatures(M);
+  EmitFunctionAttributes(M);
 }
 
 void WebAssemblyAsmPrinter::EmitProducerInfo(Module &M) {
@@ -556,6 +559,49 @@
   OutStreamer->popSection();
 }
 
+void WebAssemblyAsmPrinter::EmitFunctionAttributes(Module 

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Suggested edit to the commit description:
"use musl and stddef.h at the same time" -> "use musl and clang's stddef.h at 
the same time"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added subscribers: sbc100, sunfish.
dschuff added a comment.

The 2 common WebAssembly toolchain variants (Emscripten and wasi-sdk) use libcs 
that are derived from musl (a subset along with additions specific to their 
environments); they share the system-include configuration in 
`WebAssembly::AddClangSystemIncludeArgs()` in 
`clang/lib/Driver/Toolchains/WebAssembly.cpp`, which adds the ResourceDir 
before the Sysroot (and that I believe results in this include order). So we 
should maybe consider fixing that (something @aheejin and I should discuss with 
@sunfish and @sbc100 and the rest of the Emscripten and/or wasi folks).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

The 2 common WebAssembly toolchain variants (Emscripten and wasi-sdk) use libcs 
that are derived from musl (a subset along with additions specific to their 
environments); they share the system-include configuration in 
`WebAssembly::AddClangSystemIncludeArgs()` in 
`clang/lib/Driver/Toolchains/WebAssembly.cpp`, which adds the ResourceDir 
before the Sysroot (and that I believe results in this include order). So we 
should maybe consider fixing that (something @aheejin and I should discuss with 
@sunfish and @sbc100 and the rest of the Emscripten and/or wasi folks).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Although having said all that, I guess a question for @aaron.ballman or other 
clang header experts: It does seem that many std C headers in clang are 
designed to handle this kind of case using `include_next` (e.g. stdint.h and 
stdatomic.h) but not all of them (e.g. stddef.h or stdbool.h). So maybe we 
should try to make them consistent anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.
Herald added a subscriber: ormris.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

What would you think about adding a way to pass arguments through to wasm-opt 
on the command line, like we have for the linker, assembler, etc? Something 
like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and `-Wa`). That seems 
nicer than an env var, although it doesn't solve the problem of controlling 
whether to run the optimizer in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

If the SDK is distributed with the compiler and version-locked, it seems like 
it should be ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > What would you think about adding a way to pass arguments through to 
> > wasm-opt on the command line, like we have for the linker, assembler, etc? 
> > Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and 
> > `-Wa`). That seems nicer than an env var, although it doesn't solve the 
> > problem of controlling whether to run the optimizer in the first place.
> My guess here is that we don't need clang to have an option for passing 
> additional flags -- if people want to do extra special things with wasm-opt 
> on clang's output they can just run wasm-opt directly themselves. Does that 
> sound reasonable?
Maybe. But I still don't like the use of an env var for this kind of 
behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
reproducible and hermetic build behavior as it is, I definitely wouldn't want 
to worry about the environment affecting the output in addition to all the 
random flags, included files, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > sunfish wrote:
> > > dschuff wrote:
> > > > What would you think about adding a way to pass arguments through to 
> > > > wasm-opt on the command line, like we have for the linker, assembler, 
> > > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` 
> > > > and `-Wa`). That seems nicer than an env var, although it doesn't solve 
> > > > the problem of controlling whether to run the optimizer in the first 
> > > > place.
> > > My guess here is that we don't need clang to have an option for passing 
> > > additional flags -- if people want to do extra special things with 
> > > wasm-opt on clang's output they can just run wasm-opt directly 
> > > themselves. Does that sound reasonable?
> > Maybe. But I still don't like the use of an env var for this kind of 
> > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
> > reproducible and hermetic build behavior as it is, I definitely wouldn't 
> > want to worry about the environment affecting the output in addition to all 
> > the random flags, included files, etc.
> If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> program to be able to run it. We could just search for it in PATH, but that's 
> also a little dicey from a hermetic build perspective.
> 
> I borrowed "WASM_OPT" from 
> [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not a 
> fan of environment variables in general, but this way does have the advantage 
> that people can set it once, and not have to modify their Makefiles to add 
> new flags. Users can think of it as just being part of -O2 and friends.
> 
What's the usual way to locate things like external assemblers? Presumably we 
could use the same mechanism for wasm-opt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > sunfish wrote:
> > > dschuff wrote:
> > > > sunfish wrote:
> > > > > dschuff wrote:
> > > > > > What would you think about adding a way to pass arguments through 
> > > > > > to wasm-opt on the command line, like we have for the linker, 
> > > > > > assembler, etc? Something like `-Wo,-O2` (or `-Ww` or whatever; 
> > > > > > analogous to `-Wl` and `-Wa`). That seems nicer than an env var, 
> > > > > > although it doesn't solve the problem of controlling whether to run 
> > > > > > the optimizer in the first place.
> > > > > My guess here is that we don't need clang to have an option for 
> > > > > passing additional flags -- if people want to do extra special things 
> > > > > with wasm-opt on clang's output they can just run wasm-opt directly 
> > > > > themselves. Does that sound reasonable?
> > > > Maybe. But I still don't like the use of an env var for this kind of 
> > > > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to 
> > > > get reproducible and hermetic build behavior as it is, I definitely 
> > > > wouldn't want to worry about the environment affecting the output in 
> > > > addition to all the random flags, included files, etc.
> > > If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> > > program to be able to run it. We could just search for it in PATH, but 
> > > that's also a little dicey from a hermetic build perspective.
> > > 
> > > I borrowed "WASM_OPT" from 
> > > [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also 
> > > not a fan of environment variables in general, but this way does have the 
> > > advantage that people can set it once, and not have to modify their 
> > > Makefiles to add new flags. Users can think of it as just being part of 
> > > -O2 and friends.
> > > 
> > What's the usual way to locate things like external assemblers? Presumably 
> > we could use the same mechanism for wasm-opt?
> It checks the COMPILER_PATH environment variable and -B command-line flags, 
> which I'm not sure we should use here, but it looks like it does fall back to 
> checking PATH at the end.
> 
> So, what would you think of just checking PATH for wasm-opt?
I suspect we'll end up with -B flags if/when people start building interesting 
or nontrivial toolchains with clang (or we try to make emscripten more 
standardish), but I'm fine with leaving that out for now. Checking PATH for 
wasm-opt seems fine to me to locate the binary. Did you have that in mind as 
also the way to determine whether or not to run wasm-opt (i.e. run if it's 
there, don't if it's not)? That seems slightly error-prone in the sense that 
there would be a silent behavior change if something went wrong (e.g. wasm-opt 
goes missing) but in a world where most clang users are using wasm-opt then 
using wasm-opt by default seems reasonable; so that seems fine as a way to 
start out.

I do think we will eventually want some way to modify the behavior of wasm-opt 
though. For that matter wasm-opt might at some point become able to optimize 
object files (allowing faster links at the cost of less LTO-optimized binaries; 
we'd run a reduced set of passes post-link or none at all). In that case 
there'd also have to be further changes if we want builtin support for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

LGTM on the approach, just one more question on the wasm-opt flags.




Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:105
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();

This chain is slightly confusing. I assume this `getValue()` captures the 
number in `-O3`, `-O2`, etc? But why then do we need special-casing for 0 and 4 
above?

For that matter, we should probably not run wasm-opt at all if the opt-level is 
0, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added a comment.
This revision is now accepted and ready to land.

WRT the LTO directory name, there's theoretically the danger that someone (e.g. 
emscripten) could be doing a rolling release of the compiler and get 
invalidated within a major revision. But in that case, 1) they should be 
rebuilding their libraries on each release anyway, and 2) last time I checked, 
policy was to make auto-upgrade of bitcode work within a revision. So it's 
probably not a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

Also if at some point we are able to remove a bunch of the driver logic from 
emcc and move it here, (e.g. running clang to link instead of lld directly) 
we'll need to watch out for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: llvm/test/MC/WebAssembly/import-module.s:13
+  .import_module  foo, bar
+  .import_name  foo, qux
+

What should happen if there's a directive that refers to a symbol that doesn't 
exist in the asm file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70877



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