This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Update wasm target triple now that its the default
AbandonedPublic

Authored by sbc100 on Jan 23 2018, 11:04 AM.

Event Timeline

sbc100 created this revision.Jan 23 2018, 11:04 AM

To make sure I didn't miss anything, this is just removing the "-wasm" parts now that that's the default, right?

sbc100 retitled this revision from [WebAssembly] Update wasm target tripple now that its the default to [WebAssembly] Update wasm target triple now that its the default.Jan 23 2018, 11:14 AM

Exactly.

BTW, do you think we should also drop one or both of the "-unknown"? I'm not sure about there.. there seems to be of precedence for "foo-unknown" in the existing tests:

sbc.sbc1 (~/dev/wasm/llvm) $ git grep "triple=[^\-]* " test/    | wc -l
603
sbc.sbc1 (~/dev/wasm/llvm) $ git grep "triple=[^\-]*-unknown " test/   | wc -l
537
sbc.sbc1 (~/dev/wasm/llvm) $ git grep "triple=[^\-]*-unknown-unknown " test/   | wc -l
2166

Seems like the "-unknown-unknown" is normally explicit, and that single element triples are comparatively rare.. I'm not sure.

The codegen tests in LLVM all still use all 4 parts, and I tend to prefer that, because it avoids problems where tests pass on some hosts and fail on others due to LLVM completing the triple by auto-detecting things from the host it's running on. This may not affect wasm today, but it's a habit I'm in from other LLVM work. However, if there are reasons to omit some of the parts in the lld tests, I'm not opposed either.

Yes, I actually have another change to update the llvm side. However, in light of your comments I guess I'll hold off.

Maybe once we drop the ELF output comletely we can remove this part?

Also, how do you feel about passing the -mtripple to llc vs embedding the triple in the bitcode text? I see both done and I'd like to be consistent.

Maybe once we drop the ELF output comletely we can remove this part?

Sure, that's probably harmless.

Also, how do you feel about passing the -mtripple to llc vs embedding the triple in the bitcode text? I see both done and I'd like to be consistent.

Most of the codegen tests embed the triple in the code, because I have found it convenient to run things like "llc test/CodeGen/WebAssembly/i32.ll" to see what the code looks like. That said, this doesn't always work, because some tests need to pass additional arguments, and some tests have multiple RUN lines to test multiple targets. So I'd be ok either way.

I was going to say that most llc tests in LLVM don't use mtriple and that we should be consistent with that, but a quick grep suggests that something like 65% of the RUN lines with llc actually do contain mtriple. So I guess we could go either way. I think we still probably want to keep the triple in the bitcode (it is more convenient to run just llc, but only marginally so), in which case mtriple would be redundant. But I don't have strong opinions.

sbc100 abandoned this revision.Jan 23 2018, 1:02 PM

OK, lets try a different approach https://reviews.llvm.org/D42438