Issue 1024073002: LLVM: add support for asmjs arch and Emscripten OS

Issue 1024073002: LLVM: add support for asmjs arch and Emscripten OS (Closed)

Created:
5 years, 9 months ago by JF

Modified:
5 years, 9 months ago

Reviewers:
azakai, jvoung (off chromium), sunfish

CC:
native-client-reviews_googlegroups.com

Base URL:
https://chromium.googlesource.com/native_client/pnacl-llvm.git@master

Target Ref:
refs/heads/master

Visibility:
Public.

More Reviews

Description

LLVM: add support for asmjs arch and Emscripten OS This only adds support for the arch/OS and doesn't allow anything else in LLVM for now. There's a corresponding clang patch to allow IR generation. clang patch: https://codereview.chromium.org/1022123003 R=jvoung@chromium.org, azakai@mozilla.com, sunfish@mozilla.com BUG= https://code.google.com/p/nativeclient/issues/detail?id=4102 TEST= make check-all Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-llvm.git;a=commit;h=a508ed2cd2f86a4e055aaeb3761d6721f2d388a6

Patch Set 2 : Fix indentation. #

Total comments: 4

Patch Set 3 : Address comments. #

Created: 5 years, 9 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+43 lines, -0 lines)			Patch
	M	include/llvm/ADT/Triple.h	View	1 2	4 chunks	+33 lines, -0 lines	0 comments	Download
	M	lib/Support/Triple.cpp	View	1 2	9 chunks	+10 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

jvoung (off chromium)

Otherwise LGTM https://codereview.chromium.org/1024073002/diff/20001/include/llvm/ADT/Triple.h File include/llvm/ADT/Triple.h (right): https://codereview.chromium.org/1024073002/diff/20001/include/llvm/ADT/Triple.h#newcode381 include/llvm/ADT/Triple.h:381: bool isOSWindows() const { return false; } ...

5 years, 9 months ago (2015-03-20 17:13:38 UTC) #2

Committed patchset #3 (id:40001) manually as a508ed2cd2f86a4e055aaeb3761d6721f2d388a6 (presubmit successful).

5 years, 9 months ago (2015-03-20 17:46:16 UTC) #3

5 years, 9 months ago (2015-03-20 17:47:12 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/1024073002/diff/20001/include/llvm/ADT/Triple.h
File include/llvm/ADT/Triple.h (right):

https://codereview.chromium.org/1024073002/diff/20001/include/llvm/ADT/Triple...
include/llvm/ADT/Triple.h:381: bool isOSWindows() const { return false; }
On 2015/03/20 17:13:38, jvoung wrote:
> May want to add isOSEmscripten() here and below.

Done, and added to the ifdef above.

https://codereview.chromium.org/1024073002/diff/20001/lib/Support/Triple.cpp
File lib/Support/Triple.cpp (right):

https://codereview.chromium.org/1024073002/diff/20001/lib/Support/Triple.cpp#...
lib/Support/Triple.cpp:157: case Emscripten: return "enscripten"; // @LOCALMOD
Emscripten
On 2015/03/20 17:13:38, jvoung wrote:
> enscripten -> emscripten

Done.

Expand Messages | Collapse Messages