Page MenuHomePhabricator

dschuff (Derek Schuff)
Yet Another Compiler Compiler

Projects

User does not belong to any projects.

User Details

User Since
Jan 7 2013, 9:35 AM (314 w, 1 d)

US-Pacific timezone.

Recent Activity

Yesterday

dschuff accepted D56762: [WebAssembly] Store section alignment as a power of 2.
Tue, Jan 15, 5:35 PM
dschuff accepted D56758: [WebAssembly] Store section alignment as a power of 2.

This code LGTM modulo whatever version number we settle on in the tool-conventions issue.

Tue, Jan 15, 4:52 PM
dschuff added a comment to D56742: [WebAssembly] Parse llvm.ident into producers section.

As a follow-on, I guess we could get ambitious and add the "language" field as well. I think the only way to do that would be to sniff the language field of the DICompileUnit metadata node (it would of course only exist if there's debug info, but doing anything else seems a bit crazytown).

Tue, Jan 15, 2:33 PM
dschuff added a comment to D56684: [WebAssembly] Fixed objdump not parsing function headers..

Yeah, thinking about this a little more, I think it makes sense. IIRC the "address" (or value of a symbol, which I think is equivalent?) in the object file/linking/executable context refers to the section offset rather than memory-mapped address even for ELF, which would make our objdump consistent with other targets too. For the purposes of LLVM internals, I think you are right that having that consistency seems best; for the users, it's possible we'll want to put some more info in the final displayed output, but we can figure out how to get that once the guts are hooked up.

Tue, Jan 15, 11:05 AM
dschuff committed rL351213: [WebAssembly] Update release notes.
[WebAssembly] Update release notes
Tue, Jan 15, 9:58 AM
dschuff closed D56681: [WebAssembly] Update release notes.
Tue, Jan 15, 9:58 AM

Mon, Jan 14

dschuff added a comment to D56684: [WebAssembly] Fixed objdump not parsing function headers..

Oh I had actually forgotten that the PC offset is the offset in the module, not the offset within the function. So for objdump either way would be ok, since you could easily find what function a particular instruction is in. I guess my point is not that the formats have to match, (since e.g. you probably wouldn't be using the same code to parse both browser output and tool output?) but if you are debugging and looking at a stack trace, you should easily be able to find the same location in an objdump-produced assembly dump.... Without a calculator (annoyance of mismatched hex/dec address formats was what originally led me to add that section to the spec :) )

Mon, Jan 14, 5:10 PM
dschuff added a comment to D56684: [WebAssembly] Fixed objdump not parsing function headers..

Is this actually what we want?
If I'm debugging a crash in a browser, the browser shows stack traces by function index rather than file offset, in this format
${url}:wasm-function[${funcIndex}]:${pcOffset}
(see https://webassembly.github.io/spec/web-api/index.html#conventions)
So it might be better for objdump to match that format.

Mon, Jan 14, 5:05 PM
dschuff created D56681: [WebAssembly] Update release notes.
Mon, Jan 14, 1:28 PM
dschuff accepted D56553: [WebAssembly] Support multilibs for wasm32 and add a wasm OS that uses it.
Mon, Jan 14, 1:12 PM
dschuff added a comment to D56553: [WebAssembly] Support multilibs for wasm32 and add a wasm OS that uses it.

One difference between the multilib style and multi-arch style is whether any headers can be shared between the 2 arches. Historically at least, with bitness-flavored variants of the same architecture there were common headers used by both, and it was just the library directories that needed to be different. I've heard anecdotally that this is not the case for ARM though.

Mon, Jan 14, 10:30 AM
dschuff accepted D56645: [WebAssembly] Remove old intrinsics.
Mon, Jan 14, 8:55 AM

Fri, Jan 11

dschuff added a comment to D56553: [WebAssembly] Support multilibs for wasm32 and add a wasm OS that uses it.

LGTM but is there really no test for this? I'd expect to see this on linker command lines or something, right? If not we should add one.

Fri, Jan 11, 1:21 PM
dschuff added a comment to D56094: [WebAssembly] Fix stack pointer store check in RegStackify.

Could this CL be the cause of the newly-passing test in https://ci.chromium.org/p/wasm/builders/luci.wasm.ci/linux/2560 ?

Fri, Jan 11, 1:17 PM

Wed, Jan 9

dschuff added a comment to D48404: Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion.

Just ran across this again. @jgravelle-google is this still relevant?

Wed, Jan 9, 3:36 PM
dschuff accepted D56142: [WebAssembly] Print a debug message at the start of each pass.
Wed, Jan 9, 2:43 PM
dschuff added inline comments to D56091: [WebAssembly] Don't add IMPLICIT_DEFs in PrepareForLiveIntervals.
Wed, Jan 9, 10:33 AM

Tue, Jan 8

dschuff accepted D56093: [WebAssembly] Rename StoreResults to MemIntrinsicResults.
Tue, Jan 8, 11:30 AM

Mon, Jan 7

dschuff accepted D56094: [WebAssembly] Fix stack pointer store check in RegStackify.

This patch looks fine with the comment nit. I'm a little unclear because get_global and set_global are still marked as mayLoad and mayStore. Is it a problem with both the explicit modeling of the stack pointer and treating them as reads and writes for this purpose?

Mon, Jan 7, 10:28 AM

Wed, Dec 19

dschuff added a comment to D52634: [WebAssembly] Add DBG_VALUE with local operands location in WebAssemblyExplicitLocals pass.

Come to think of it, might we actually want the possibility of constant offset in addition to the "kind" and index field, e.g. for cases where the value is actually a pointer?

Wed, Dec 19, 10:37 AM · debug-info
dschuff added a comment to D52634: [WebAssembly] Add DBG_VALUE with local operands location in WebAssemblyExplicitLocals pass.

I think there will eventually be more consumers, including traditional-style debuggers (even if just for non-web use cases).
I had initially hoped that we could get away with just using DWARF registers more-or-less directly for locals, but as Yury pointed out, there are a lot of places that assume a fixed number of architectural registers. And of course wasm globals and the wasm value stack don't really have analogs on most architectures. I still hope we'll be able to use DWARF's CFA and other stack stuff for the stack in linear memory. (Although in our case the stack pointer will be a global rather than a register. Hopefully we can make that work.)

Wed, Dec 19, 10:28 AM · debug-info

Tue, Dec 18

dschuff added a comment to D52634: [WebAssembly] Add DBG_VALUE with local operands location in WebAssemblyExplicitLocals pass.

@aprantl Is the advantage of your suggested approach just that we don't have to define a new expression type? Obviously the interpretation is not the same as DW_OP_breg on other targets so as you say, either way there would have to be special logic in all the tools that consume it. Is this kind of repurposing of builtin primitives common?

Tue, Dec 18, 8:43 PM · debug-info

Mon, Dec 17

dschuff accepted D55797: [WebAssembly] Make assembler check for proper nesting of control flow..
Mon, Dec 17, 5:09 PM

Dec 10 2018

dschuff accepted D55347: [WebAssembly] TargetStreamer cleanup (NFC).

At a high level (i.e. across all the LLVM targets) the intention is that binary and asm streamers have the same interface. That's sort of enforced by the fact that they both have the same base class, but I guess in actuality the "interface" also includes the data on the symbol object. I think I agree that it makes more sense to think of the symbols as owned outside the streamer and have them be a way to pass info into the streamer (the current code is more like having them be state that is only needed by the binary streamer but which lives outside the streamer). I guess that's also another way of saying that symbol type and module name are properties of a symbol even if those properties are only needed by one of the streamers. I guess it might also be useful for debugging or future modifications to have the contents of the symbols be the same regardless of which output type is in use.

Dec 10 2018, 4:06 PM
dschuff added inline comments to D55401: [WebAssembly] Fix assembler parsing of br_table..
Dec 10 2018, 2:29 PM
dschuff accepted D55526: Fix LLVM_LINK_LLVM_DYLIB build of TextAPI.

I don't 100% understand the distinction but if you've tested with LLVM_LINK_LLVM_DYLIB and BUILD_SHARED_LIBS and the static build then LGTM :)

Dec 10 2018, 2:01 PM

Dec 7 2018

dschuff added inline comments to D55401: [WebAssembly] Fix assembler parsing of br_table..
Dec 7 2018, 1:45 PM

Dec 3 2018

dschuff added inline comments to D55149: [WebAssembly] Enforce assembler emits to streamer in order..
Dec 3 2018, 10:10 AM

Nov 28 2018

dschuff added inline comments to D54873: [WebAssembly] Make signature index not a part of EventType.
Nov 28 2018, 10:59 AM

Nov 27 2018

dschuff added inline comments to D54873: [WebAssembly] Make signature index not a part of EventType.
Nov 27 2018, 4:44 PM
dschuff added inline comments to D54873: [WebAssembly] Make signature index not a part of EventType.
Nov 27 2018, 4:40 PM

Nov 19 2018

dschuff added a comment to D54661: [WebAssembly] Don't override default implementation of isOffsetFoldingLegal. NFC..

Yeah I think for PIC code all globals need to be relative.

Nov 19 2018, 5:41 PM
dschuff added a comment to D54683: [WebAssembly] Delete unused using statements (NFC).

LLVM doesn't actually have an IWYU guideline though: https://llvm.org/docs/CodingStandards.html#include-as-little-as-possible ... but also not a "dont IWYU".

Nov 19 2018, 5:31 PM
dschuff added inline comments to D54689: [WebAssembly] Rename function types to signatures.
Nov 19 2018, 5:23 PM
dschuff added a comment to D54661: [WebAssembly] Don't override default implementation of isOffsetFoldingLegal. NFC..

Not sure I am parsing your commit message right. So we fold if DSO local, and not otherwise?

Nov 19 2018, 4:29 PM
dschuff added inline comments to D54689: [WebAssembly] Rename function types to signatures.
Nov 19 2018, 4:25 PM

Nov 16 2018

dschuff accepted D54652: [WebAssembly] replaced .param/.result by .functype.

otherwise LGTM!

Nov 16 2018, 4:12 PM
dschuff accepted D54644: [WebAssembly] Cleanup unused declares in test code.
Nov 16 2018, 12:39 PM
dschuff accepted D54637: [WebAssembly] Default to static reloc model.
Nov 16 2018, 10:45 AM

Nov 15 2018

dschuff added inline comments to D54571: [WebAssembly] Split BBs after throw instructions.
Nov 15 2018, 3:14 PM
dschuff accepted D54571: [WebAssembly] Split BBs after throw instructions.

Otherwise LGTM

Nov 15 2018, 1:33 PM

Nov 14 2018

dschuff accepted D54249: [WebAssembly] Initial support for shared objects (-shared).
Nov 14 2018, 10:17 AM
dschuff accepted D54490: [WebAssembly] Add support for dylink section in object format.
Nov 14 2018, 10:11 AM

Nov 13 2018

dschuff added inline comments to D54249: [WebAssembly] Initial support for shared objects (-shared).
Nov 13 2018, 5:40 PM
dschuff accepted D54447: [WebAssembly] Fix broken assumption that all bitcasts are to functions types.
Nov 13 2018, 11:09 AM
dschuff accepted D54096: [WebAssembly] Add support for the event section.

LGTM too. I realized I don't really like (or perhaps I just don't understand) the term "Attribute" where it's used in the spec, but it's fine for this code to match that.

Nov 13 2018, 10:24 AM

Nov 12 2018

dschuff added inline comments to D54447: [WebAssembly] Fix broken assumption that all bitcasts are to functions types.
Nov 12 2018, 3:38 PM
dschuff added a comment to D54449: [compiler-rt][builtins][PowerPC] Enable builtins tests on PowerPC 64 bit LE .

Conceptually, COMPILER_RT_ABI is the calling convention used by all compiler-rt functions. It is explicitly used everywhere because it may be different from the default calling convention on some targets. In practice I believe it only affects ARM, where the calling convention uses ARM's soft-float ABI (where IIRC floats are passed in integer registers) even on targets that use the hard-float ABI by default. The compiler knows the calling conventions and emits the right code when it emits the libcalls, and the ABI declaration on the implementations causes the right calling convention to be used there. The unusual thing about the tests is that they locally declare the compiler-rt functions explicitly. So this declaration has to match the implementation. It may not matter for PPC but my recommendation would be "just use it everywhere in compiler-rt" for simplicity.

Nov 12 2018, 3:20 PM

Nov 9 2018

dschuff accepted D54360: [WebAssembly] Added WasmAsmParser..

otherwise lgtm

Nov 9 2018, 4:25 PM
dschuff accepted D54353: [WebAssembly] Disable custom NaN payload tests.
Nov 9 2018, 1:58 PM

Nov 8 2018

dschuff added inline comments to D54249: [WebAssembly] Initial support for shared objects (-shared).
Nov 8 2018, 10:49 AM

Nov 7 2018

dschuff added inline comments to D54096: [WebAssembly] Add support for the event section.
Nov 7 2018, 3:11 PM
dschuff added inline comments to D54096: [WebAssembly] Add support for the event section.
Nov 7 2018, 1:21 PM

Nov 6 2018

dschuff committed rL346249: [WebAssembly] Address review comments from r346248 [NFC].
[WebAssembly] Address review comments from r346248 [NFC]
Nov 6 2018, 10:05 AM
dschuff committed rLLD346249: [WebAssembly] Address review comments from r346248 [NFC].
[WebAssembly] Address review comments from r346248 [NFC]
Nov 6 2018, 10:05 AM
dschuff committed rLLD346248: [WebAssembly] Support creation and import of shared memories.
[WebAssembly] Support creation and import of shared memories
Nov 6 2018, 10:03 AM
dschuff committed rL346248: [WebAssembly] Support creation and import of shared memories.
[WebAssembly] Support creation and import of shared memories
Nov 6 2018, 10:03 AM
dschuff closed D54130: [WebAssembly] Support creation and import of shared memories.
Nov 6 2018, 10:03 AM
dschuff committed rL346246: [WebAssembly] Add shared memory support to limits field.
[WebAssembly] Add shared memory support to limits field
Nov 6 2018, 9:30 AM
dschuff closed D54131: [WebAssembly] Add shared memory support to limits field.
Nov 6 2018, 9:30 AM

Nov 5 2018

dschuff added a comment to D54131: [WebAssembly] Add shared memory support to limits field.

Object files currently import their memories, right? We should probably make that import shared when we use thread-model=posix.

Nov 5 2018, 5:40 PM
dschuff added a parent revision for D54130: [WebAssembly] Support creation and import of shared memories: D54131: [WebAssembly] Add shared memory support to limits field.
Nov 5 2018, 5:38 PM
dschuff added a child revision for D54131: [WebAssembly] Add shared memory support to limits field: D54130: [WebAssembly] Support creation and import of shared memories.
Nov 5 2018, 5:38 PM
dschuff removed a child revision for D54130: [WebAssembly] Support creation and import of shared memories: D54131: [WebAssembly] Add shared memory support to limits field.
Nov 5 2018, 5:38 PM
dschuff removed a parent revision for D54131: [WebAssembly] Add shared memory support to limits field: D54130: [WebAssembly] Support creation and import of shared memories.
Nov 5 2018, 5:38 PM
dschuff added a parent revision for D54131: [WebAssembly] Add shared memory support to limits field: D54130: [WebAssembly] Support creation and import of shared memories.
Nov 5 2018, 5:38 PM
dschuff added a child revision for D54130: [WebAssembly] Support creation and import of shared memories: D54131: [WebAssembly] Add shared memory support to limits field.
Nov 5 2018, 5:38 PM
dschuff created D54131: [WebAssembly] Add shared memory support to limits field.
Nov 5 2018, 5:36 PM
dschuff created D54130: [WebAssembly] Support creation and import of shared memories.
Nov 5 2018, 5:33 PM
dschuff added inline comments to D54096: [WebAssembly] Add support for the event section.
Nov 5 2018, 10:17 AM

Nov 2 2018

dschuff accepted D53842: [WebAssembly] Parsing missing directives to produce valid .o.

I like the idea of trying to make the assembler less stateful if possible, but that can be another CL

Nov 2 2018, 10:38 AM
dschuff updated subscribers of D53842: [WebAssembly] Parsing missing directives to produce valid .o.
Nov 2 2018, 10:38 AM

Nov 1 2018

dschuff accepted D54012: [WebAssembly] Added a .globaltype directive to .s output..
Nov 1 2018, 5:01 PM
dschuff accepted D53819: [WebAssembly] Fix bugs in rethrow depth counting and InstPrinter.
Nov 1 2018, 3:55 PM
dschuff added a comment to D53819: [WebAssembly] Fix bugs in rethrow depth counting and InstPrinter.

By "Multiple catches for a try" you mean catches with different tags?

Nov 1 2018, 3:54 PM
dschuff accepted D53396: [WebAssembly] Fixup main signature by default.

At some point I could imagine a frontend possibly wanting to opt out of this pass entirely, but we can always add that option in the future.

Nov 1 2018, 12:30 PM
dschuff added inline comments to D53842: [WebAssembly] Parsing missing directives to produce valid .o.
Nov 1 2018, 10:10 AM

Oct 29 2018

dschuff added inline comments to D53842: [WebAssembly] Parsing missing directives to produce valid .o.
Oct 29 2018, 4:25 PM
dschuff added inline comments to D53842: [WebAssembly] Parsing missing directives to produce valid .o.
Oct 29 2018, 4:07 PM

Oct 25 2018

dschuff accepted D53722: [WebAssembly] Lower to target-independent saturating add.
Oct 25 2018, 11:48 AM
dschuff accepted D53721: [WebAssembly] Use target-independent saturating add.
Oct 25 2018, 11:48 AM

Oct 24 2018

dschuff added a comment to D53620: [WebAssembly] Error out when block/loop markers mismatch.

I think assertions here are OK as long as the plan of record is is to put error handling into the parsers. If we have assertions, we can't have a test case (until we can test the parser errors) because it would always fail on release builds without asserts.

Oct 24 2018, 4:10 PM
dschuff accepted D53619: [WebAssembly] Fix immediate of rethrow when throwing to caller.
Oct 24 2018, 4:08 PM
dschuff accepted D53634: [WebAssembly] Support EH instructions in InstPrinter.
Oct 24 2018, 9:38 AM
dschuff added a comment to D53620: [WebAssembly] Error out when block/loop markers mismatch.

I think catching block mismatch errors at parse time makes sense; although the third way that MCInst can be generated (aside from the compiler and asmparser) is the object file parser. So those checks would have to go both in the asm parser and object file parser. But if that means that we can hook into some existing error handling mechanism instead of using report_fatal_error, then it's probably worth it.
I agree that we can't use an assert string in a CHECK. The test wouldn't pass in a release non-asserts build (our release binaries have assertions, but it's not the default, and the official release binaries don't).

Oct 24 2018, 8:51 AM

Oct 23 2018

dschuff added a comment to D53620: [WebAssembly] Error out when block/loop markers mismatch.

This kind of code should never be produced by the wasm backend, right? In which case this could be an assert/unreachable rather than a fatal error. But I guess this code also prints insts that are parsed from an obj or asm file, right? In that case the input could just be bogus and we shouldn't assert.
So I'm not sure if it's possible here to tell which one to ideally use.
But also:

  1. can this test be an llvm-mc test (which has assembly input and prints assembly)? I think the answer is "yes" (at least eventually) but I'm not sure if the asm parser is far enough along yet for that.
  2. I wonder if there's some kind of infrastructure in MC for printing errors (e.g. for malformed asm) other than just the big hammer of report_fatal_error? If so it probably makes sense to use that (even if we can't tell the difference between bogus code from the compiler and bogus input from an asm user).
Oct 23 2018, 5:32 PM

Oct 19 2018

dschuff added inline comments to D53307: [WebAssembly][NFC] Remove WebAssemblyStackifier TableGen backend.
Oct 19 2018, 10:44 AM

Oct 11 2018

dschuff accepted D52748: [WebAssembly] LSDA info generation.
Oct 11 2018, 1:23 PM

Oct 9 2018

dschuff closed D52580: Refactor WasmSignature and use it for MCSymbolWasm.

This was landed in rL343733. not sure why it didn't close already.

Oct 9 2018, 4:11 PM
dschuff added a comment to D52748: [WebAssembly] LSDA info generation.

Is that information what's used/needed to allow the linker to create a separate wasm segment per global?

Oct 9 2018, 2:33 PM
dschuff accepted D52899: [TTI] Check that lowered type is floating point before calling isFabsFree.

Test looks much nicer now, LGTM

Oct 9 2018, 11:05 AM

Oct 4 2018

dschuff added a comment to D52899: [TTI] Check that lowered type is floating point before calling isFabsFree.

Right but the point is that we don't currently have a way to ensure that the code is doing its job; if the SLP vectorizer stops doing its thing and calling the code that was asserting, then this test won't be testing anything.

Oct 4 2018, 5:38 PM
dschuff added a comment to D52856: [WebAssembly] __builtin_wasm_replace_lane_* builtins.

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.

Oct 4 2018, 3:13 PM
dschuff accepted D52902: [WebAssembly] Don't modify preds/succs iterators while erasing from them.

Otherwise LGTM, and I verified that it fixes the expensive checks failure.

Oct 4 2018, 1:44 PM
dschuff added inline comments to D52902: [WebAssembly] Don't modify preds/succs iterators while erasing from them.
Oct 4 2018, 1:42 PM
dschuff added a comment to rL343746: [WebAssembly] Add WebAssembly to LLVM_ALL_TARGETS.

Yes, I can reproduce. I'll take a look.

Oct 4 2018, 11:52 AM

Oct 3 2018

dschuff committed rL343746: [WebAssembly] Add WebAssembly to LLVM_ALL_TARGETS.
[WebAssembly] Add WebAssembly to LLVM_ALL_TARGETS
Oct 3 2018, 4:58 PM
dschuff closed D52850: [WebAssembly] Add WebAssembly to LLVM_ALL_TARGETS.
Oct 3 2018, 4:58 PM
dschuff created D52850: [WebAssembly] Add WebAssembly to LLVM_ALL_TARGETS.
Oct 3 2018, 4:03 PM
dschuff committed rLLD343734: [WebAssembly] Refactor use of signatures.
[WebAssembly] Refactor use of signatures
Oct 3 2018, 3:28 PM