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 (280 w, 5 d)

US-Pacific timezone.

Recent Activity

Thu, May 17

dschuff added inline comments to D47005: [WebAssembly] Add functions for EHScopes.
Thu, May 17, 2:46 PM
dschuff added inline comments to D47005: [WebAssembly] Add functions for EHScopes.
Thu, May 17, 1:02 PM
dschuff added a comment to D43746: [WebAssembly] Add Wasm exception handling prepare pass.

The Wasm code looks good with the one comment so LGTM if @majnemer is OK with the modification to WinEHPrepare.

Thu, May 17, 9:30 AM
dschuff accepted D45559: [WebAssembly] Add Wasm personality and isScopedEHPersonality().

Thanks for your patience on this, I really do think this is a big readability improvement.

Thu, May 17, 9:14 AM

Wed, May 16

dschuff accepted D46969: [WebAssembly] Remove unused headers in MCWasmObjectWriter.
Wed, May 16, 2:33 PM
dschuff added a comment to D46977: [NFC] WebAssembly build fix.

LGTM, thanks

Wed, May 16, 2:30 PM

Tue, May 15

dschuff accepted D46556: [MC] Move MCAssembler::dump into the correct cpp file. NFC.

Also if you use the LINK_LLVM_DYLIB CMake mode (as our bots do) you should test the regular non-dylib static link build.

Tue, May 15, 3:00 PM
dschuff accepted D45796: [WebAssembly] Support imports from custom module names.
Tue, May 15, 2:55 PM

Thu, May 10

dschuff added inline comments to D44090: [WebAssembly] Support instruction selection for catching exceptions.
Thu, May 10, 5:13 PM
dschuff added inline comments to D43746: [WebAssembly] Add Wasm exception handling prepare pass.
Thu, May 10, 4:33 PM
dschuff added a comment to D45559: [WebAssembly] Add Wasm personality and isScopedEHPersonality().

Let me clarify. So there are the two options:

  1. Use 'funclet' to only mean outlined funclet in AsmPrinter I guess @dschuff endorses this, right? I'm not sure what @majnemer meant. Actually this is what I first tried to do in this patch, but this seems to be a harder way now, because as I said, the term funclet is used to mean a lot of things and being used everywhere including both llvm and clang in all sorts of method names in comments, not only this personality functions.
  2. Leave the uses of 'funclet' as they are, delete usesWindowsEHInstructions function and change the uses of them back to isFuncletEHPersonality, and create something like usesFuncletLSDA to be used in the cases that only pertain to real outlined funclets. This seems to be less work, because we don't need to change the uses of 'funclet' everywhere...
Thu, May 10, 3:30 PM
dschuff added inline comments to D45848: [WebAssembly] Initial Disassembler..
Thu, May 10, 2:29 PM
dschuff accepted D46561: [WebAssembly] Create section start symbols automatically for all sections.
Thu, May 10, 10:31 AM
dschuff accepted D46342: [WebAsembly] Update default triple in test files to wasm32-unknown-unkown..
Thu, May 10, 10:27 AM
dschuff added a comment to D45559: [WebAssembly] Add Wasm personality and isScopedEHPersonality().

But currently the term 'use funclets', both in function names and comments / error messages, is being used to mean both 'use windows EH IR' and 'use outlined funclets' in the code.
Currently 'use funclet' can mean either thing in LLVM. But in wasm we only want the IR level behavior but not the outlining behavior. An example of the latter, which deals with the real function outlining, is this code preparing information for LSDA generation for funclets, which we are not gonna use in wasm.

Thu, May 10, 8:26 AM

Mon, May 7

dschuff added inline comments to D45559: [WebAssembly] Add Wasm personality and isScopedEHPersonality().
Mon, May 7, 5:32 PM
dschuff accepted D46555: [WebAssembly] MC: Use existing MCSymbol.Index field rather than inventing extra mapping.
Mon, May 7, 5:05 PM
dschuff added a comment to D45848: [WebAssembly] Initial Disassembler..

haven't looked at most of the code, but thought about the register operand issue.

Mon, May 7, 4:54 PM
dschuff accepted D46289: [MC] ELFObjectWriter: Removing unneeded variable and cast. NFC..
Mon, May 7, 12:13 PM

Apr 6 2018

dschuff accepted D45386: [WebAssembly] Enabled -triple=wasm32-unknown-unknown-wasm path using ELF directive parser..
Apr 6 2018, 2:14 PM

Apr 5 2018

dschuff added inline comments to D44184: Write DWARF data into WASM object file.
Apr 5 2018, 4:20 PM

Apr 4 2018

dschuff accepted D45280: WebAssembly: Never write more than 32-bits for WebAssembly::OPERAND_OFFSET32.

LGTM for the encoding... I hope the -1 offset wasn't being generated by the compiler though?

Apr 4 2018, 1:02 PM

Apr 2 2018

dschuff accepted D45183: [CodeGen]Add NoVRegs property on PostRASink and ShrinkWrap.
Apr 2 2018, 1:47 PM

Mar 30 2018

dschuff accepted D45103: [WebAssembly] Register wasm passes with the PassRegistry.
Mar 30 2018, 1:32 PM
dschuff committed rL328876: [WebAssembly] Refactor tablegen for store instructions (NFC).
[WebAssembly] Refactor tablegen for store instructions (NFC)
Mar 30 2018, 10:05 AM
dschuff closed D45064: [WebAssembly] Refactor tablegen for store instructions (NFC).
Mar 30 2018, 10:05 AM

Mar 29 2018

dschuff created D45064: [WebAssembly] Refactor tablegen for store instructions (NFC).
Mar 29 2018, 2:35 PM

Mar 28 2018

dschuff added inline comments to D44931: [WebAssembly] Use Windows EH instructions for Wasm EH.
Mar 28 2018, 4:20 PM
dschuff added a comment to D44931: [WebAssembly] Use Windows EH instructions for Wasm EH.

Otherwise it looks good to me, although @majnemer would know more about the subtleties of what IR actually gets generated.

Mar 28 2018, 3:59 PM

Mar 27 2018

dschuff added inline comments to D44931: [WebAssembly] Use Windows EH instructions for Wasm EH.
Mar 27 2018, 5:16 PM

Mar 21 2018

dschuff updated subscribers of rL328112: [WebAssembly] Suppress unused function warning for register name matcher.

Sorry about that. Although I'm confused though why setting 'ShouldEmitMatchRegisterName' (which I added right before commit when I saw this warning myself) (https://github.com/llvm-mirror/llvm/blob/master/lib/Target/WebAssembly/WebAssembly.td#L84) didn't work.

Mar 21 2018, 9:29 AM

Mar 20 2018

dschuff committed rL328057: [WebAssembly] Update torture compile test expectations.
[WebAssembly] Update torture compile test expectations
Mar 20 2018, 4:03 PM
dschuff committed rL328049: [WebAssembly] Strip threadlocal attribute from globals in single thread mode.
[WebAssembly] Strip threadlocal attribute from globals in single thread mode
Mar 20 2018, 3:04 PM
dschuff closed D44703: [WebAssembly] Strip threadlocal attribute from globals in single thread mode.
Mar 20 2018, 3:04 PM
dschuff updated the diff for D44703: [WebAssembly] Strip threadlocal attribute from globals in single thread mode.
  • add braces in else
Mar 20 2018, 3:02 PM
dschuff added inline comments to D44703: [WebAssembly] Strip threadlocal attribute from globals in single thread mode.
Mar 20 2018, 3:02 PM
dschuff updated the summary of D44703: [WebAssembly] Strip threadlocal attribute from globals in single thread mode.
Mar 20 2018, 2:28 PM
dschuff created D44703: [WebAssembly] Strip threadlocal attribute from globals in single thread mode.
Mar 20 2018, 2:27 PM
dschuff committed rL328028: [WebAssembly] Added initial AsmParser implementation..
[WebAssembly] Added initial AsmParser implementation.
Mar 20 2018, 1:09 PM
dschuff closed D44329: [WebAssembly] Added initial AsmParser implementation..
Mar 20 2018, 1:09 PM

Mar 19 2018

dschuff accepted D44329: [WebAssembly] Added initial AsmParser implementation..

LGTM.

Mar 19 2018, 4:10 PM

Mar 15 2018

dschuff added a comment to D44329: [WebAssembly] Added initial AsmParser implementation..

I think i'd be in favor of letting this patch go in as it is for now, since it uses our existing syntax, and then have a separate discussion of how to change it on tool-conventions.

Mar 15 2018, 4:02 PM
dschuff committed rL327673: [WebAssembly] Add DebugLoc information to WebAssembly block and loop..
[WebAssembly] Add DebugLoc information to WebAssembly block and loop.
Mar 15 2018, 3:09 PM
dschuff closed D44448: Add DebugLoc information to WebAssembly block and loop..
Mar 15 2018, 3:09 PM
dschuff added inline comments to D44184: Write DWARF data into WASM object file.
Mar 15 2018, 2:43 PM
dschuff accepted D44448: Add DebugLoc information to WebAssembly block and loop..

Do you want me to commit this?

Mar 15 2018, 2:31 PM
dschuff added a comment to D44448: Add DebugLoc information to WebAssembly block and loop..

Otherwise LGTM

Mar 15 2018, 10:34 AM

Mar 14 2018

dschuff added a comment to D44316: [WebAssembly] Demangle symbol names for use by the browser debugger.

I think the symbol table (and any names that actually have meaning to the compiler and/or linker) should be mangled names, and only the name section should have demangled names. I also think it makes sense for libObject to only use the symbol table and ignore the name section. In other words, the name section should be thought of as metadata or debug info and in that sense shouldn't be "trusted" to always be correct (although of course we should preserve it wherever we can).

Mar 14 2018, 10:48 AM

Mar 13 2018

dschuff accepted D44451: [WebAssembly] Fix expected contents of relocations with addends.

LGTM, and confirmed that it makes libcxx configure successfully.

Mar 13 2018, 5:50 PM
dschuff added a comment to D44427: [WebAssembly] Add export/import for function pointer table.

I don't think import or export should be on by default (even if emscripten decides to do that) but the option should be there. It's still pretty early days for the VMs and just because they don't optimize better today doesn't mean they never will.

Mar 13 2018, 9:10 AM

Mar 12 2018

dschuff added inline comments to D44329: [WebAssembly] Added initial AsmParser implementation..
Mar 12 2018, 5:40 PM
dschuff added a comment to D44329: [WebAssembly] Added initial AsmParser implementation..

looking pretty good overall, here are a couple of high-level comments.

Mar 12 2018, 4:08 PM

Mar 9 2018

dschuff added a comment to D43744: [WebAssembly] Fix rethrow's argument type.

Sure, but once everything is in, then we shouldn't have the problem that we get undefs created. So we coudl just roll this into whatever CL adds the defs.

Mar 9 2018, 4:59 PM
dschuff added a comment to D43744: [WebAssembly] Fix rethrow's argument type.

We don't actually create uses without reaching defs though, do we? Isn't it true that by the time we run PrepareForLiveIntervals, we've lowered the catch (and the catch will create a def of the phys reg that reaches the rethrow?)

Mar 9 2018, 4:54 PM

Mar 8 2018

dschuff added a comment to D43744: [WebAssembly] Fix rethrow's argument type.

Hm, another option could be undef here. I don't know if that would be any easier though.

Mar 8 2018, 9:29 AM

Mar 7 2018

dschuff accepted D43706: [WebAssembly] Add except_ref as a first-class type.
Mar 7 2018, 4:52 PM
dschuff added a comment to D43706: [WebAssembly] Add except_ref as a first-class type.

Everything else seems pretty straightforward.

Mar 7 2018, 4:51 PM
dschuff added inline comments to D43746: [WebAssembly] Add Wasm exception handling prepare pass.
Mar 7 2018, 4:03 PM
dschuff added a comment to D43744: [WebAssembly] Fix rethrow's argument type.

I forget what the spec says right now, but I think we will need a null value for except_ref in the spec anyway, because locals are implicitly initialized to their respective 0 values. So we'd need a corresponding 0 value for ref types. So I think it would be fine to have a nullref type in LLVM too.

Mar 7 2018, 11:19 AM
dschuff accepted D43740: [WebAssembly] Add IntrNoReturn property to throw/rethrow intrinsics.
Mar 7 2018, 11:05 AM

Mar 2 2018

dschuff committed rL326593: [X86][x32] Save callee-save register used as base pointer for x32 ABI.
[X86][x32] Save callee-save register used as base pointer for x32 ABI
Mar 2 2018, 9:50 AM
dschuff closed D42358: [X86][x32] Save callee-save register used as base pointer for x32 ABI.
Mar 2 2018, 9:50 AM
dschuff added inline comments to D42358: [X86][x32] Save callee-save register used as base pointer for x32 ABI.
Mar 2 2018, 9:44 AM

Mar 1 2018

dschuff added a comment to D42358: [X86][x32] Save callee-save register used as base pointer for x32 ABI.

Sorry this slipped through; this looks right to me.

Mar 1 2018, 3:23 PM
dschuff accepted D42358: [X86][x32] Save callee-save register used as base pointer for x32 ABI.
Mar 1 2018, 3:23 PM
dschuff added a comment to D43744: [WebAssembly] Fix rethrow's argument type.

Why do we need to omit the argument here? I'm not sure I understand what you mean by adding it after isel. Can/should a pass add an argument that doesn't match the format in the td?

Mar 1 2018, 3:14 PM
dschuff added inline comments to D43706: [WebAssembly] Add except_ref as a first-class type.
Mar 1 2018, 3:13 PM
dschuff accepted D43681: [WebAssembly] Add exception handling option.

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.

Mar 1 2018, 11:13 AM
dschuff accepted D43742: [WebAssembly] Gather EH instructions in one place. NFC..
Mar 1 2018, 10:34 AM
dschuff accepted D43921: [WebAssembly] Use uint8_t for single byte values to match the spec.

I'm fine with doing this change since it does reflect the spec. Although IIRC the relevant types don't yet have any values over 128 and we've kept it that way so we can always upgrade to varint when we need to. So in principle we could keep the code using varint and then it won't have to change in the future.

Mar 1 2018, 9:56 AM

Feb 16 2018

dschuff added a comment to D43365: [WebAssembly] MC: Make explicit our current lack of support for relocations against unnamed temporary symbols..

Yeah come to think of it, I thought we just failed isel for blockaddress?

Feb 16 2018, 9:26 AM
dschuff accepted D43365: [WebAssembly] MC: Make explicit our current lack of support for relocations against unnamed temporary symbols..
Feb 16 2018, 8:47 AM

Feb 12 2018

dschuff accepted D43212: [WebAssembly] Update ADT/TripleTest.cpp now that default file format has changed.
Feb 12 2018, 3:46 PM
dschuff accepted D43205: [WebAssembly] Fix casting MCSymbol to MCSymbolWasm on ELF.
Feb 12 2018, 12:31 PM
dschuff added inline comments to D43205: [WebAssembly] Fix casting MCSymbol to MCSymbolWasm on ELF.
Feb 12 2018, 12:17 PM

Feb 9 2018

dschuff added a comment to D42576: [test-suite] Add prototypes to functions in Olden and MiBench.
  • My reading of the C standard is that using a prototype less declaration and defining that as a vararg functions is invalid in C. If you find bugs in this area we should fix them.

Interesting, I'll look closer at that. We haven't gotten any complaints about it so far.

  • This patch simply removes all prototype less functions which is not fine.

Sorry, in case I wasn't clear, I agree that these tests are valid C and we should not change them.

  • The llvm IR coming out of clang appears to mark all prototype less functions as varargs which from what I suspect makes it impossible to implement varargs different from prototype-less functions. But that is a problem in clang/IR, it doesn't change the fact that the tests are valid C.

Yeah, you can sort of cheat because in C, vararg functions must have at least one non-vararg parameter (at least AFAIK no C standard implemented by Clang allows that). So you can tell a real vararg function declare i32 @foo(i32, ...) from a non-prototype function declare i32 @foo(...). I suspect that only works for C though.

Feb 9 2018, 12:13 PM
dschuff added a comment to D42576: [test-suite] Add prototypes to functions in Olden and MiBench.

I'll just add that what we do in WebAssembly right now for unprototyped functions is to codegen them using the fixed-arg calling convention; at link-time if there are any mismatches between the definition and any calls, then the program has UB, and the resulting WebAssembly module will be invalid. I think this currently has the problem that if the definition does turn out to be vararg, then we have generated an invalid module even though the program is correct (I consider that a bug or shortcoming in our toolchain though). In any case, we do aim to support correct programs with prototypeless functions. If the test suite has non-vararg cases where there are mismatches between the callsites and/or the definition (I have seen that in some other test/benchmark suites), then even though that works on most X86 and ARM ABIs that I know of it's still UB and I'd be in favor of removing it from LLVM's test suite.

Feb 9 2018, 11:22 AM

Feb 8 2018

dschuff added a comment to D42435: [WebAssembly] Rely on default file format in test code. NFC..

I like this idea, did you abandon this CL in favor of something else?

Feb 8 2018, 11:23 AM

Feb 5 2018

dschuff added a comment to D42788: [SDAG] Legalize all CondCodes by inverting them and/or swapping operands.

So, it turns out that this actually does change functionality on targets (such as WebAssembly) that use the expansion for unordered comparisons because it tries inverting the condition before falling all the way through to line 1680. This actually results in better code for wasm; see https://reviews.llvm.org/rL324305

Feb 5 2018, 5:47 PM
dschuff updated subscribers of rL324305: [WebAssembly] Fix test expectations after r324274.

@sunfish This is really just a change-detector test for the cond code expansion behavior that changed in https://reviews.llvm.org/D42788. Should we consider loosening it so that it just tests that we used the expansion? OTOH it's a bit concerning that that change didn't affect any other tests (!)

Feb 5 2018, 5:26 PM
dschuff committed rL324305: [WebAssembly] Fix test expectations after r324274.
[WebAssembly] Fix test expectations after r324274
Feb 5 2018, 5:23 PM
dschuff accepted D42869: [WebAssembly] Remove DataSize from linking metadata.
Feb 5 2018, 1:54 PM

Feb 2 2018

dschuff added a comment to D42867: [WebAssembly] Add __data_end linker-synthetic symbol.

I do like the idea of having a linker-generated symbol instead of metadata for _end (or _wasm_end or whatever) and it's something that is unlikely to be broken in the future. So I think we should do that. But I do agree that specifying all these things would limit our ability to improve the layout in the future, especially wrt the order of the static/heap/stack bits. Since we have more constraints on our memory space than ELF (e.g. expanding only at the top instead of mmap), we might want to retain that flexibility, or even make things more dynamic or configurable, which would probably argue against matching the behavior of ELF here. We could consider having separate start/end markers for particular sections instead of having implicit ordering, if there's a use case for that.

Feb 2 2018, 4:17 PM

Jan 31 2018

dschuff added a comment to D42095: [WebAssembly] Output provisional table to match LLD relocatable output.

This change does not effect lld because the wasm "table" in
object file format is purely cosmetic and lld ignores its
contents.

Jan 31 2018, 10:49 AM
dschuff added a comment to D37831: [WebAssembly] Don't pass -ffunction-section/-fdata-sections .

After a little discussion about this and the -gc-sections linker flag, the augments for the tools having sensible defaults seem to be winning.

Jan 31 2018, 9:21 AM
dschuff accepted D37831: [WebAssembly] Don't pass -ffunction-section/-fdata-sections .
Jan 31 2018, 9:21 AM

Jan 30 2018

dschuff added a comment to D42511: [WebAssembly] Add support for --gc-sections.

I think LLD should probably default to no gc, and in general to minimal/most simple options, and the smarts should be in the driver. I think this matches other targets, and I think it also matches (or at least, should match) the way we have options the frontend/backend vs the driver; i.e. IIRC the driver has logic to pass -ffunction-sections and other codegen options to the frontend. So basically all the special logic goes in one place.

Jan 30 2018, 9:26 AM

Jan 23 2018

dschuff accepted D40526: [WebAssembly] Change size_t to `unsigned long`.

LGTM FTR, as long as we coordinate when this lands with emscripten.

Jan 23 2018, 2:45 PM
dschuff accepted D42440: [WebAssembly] MC: Use inline triple in test bitcode files.
Jan 23 2018, 2:44 PM
dschuff added a comment to D42433: [WebAssembly] Update wasm target triple now that its the default.

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.

Jan 23 2018, 12:23 PM

Jan 22 2018

dschuff added a comment to D42226: [WebAssembly] Add test expectations for gcc C++ tests (gcc/testsuite/g++.dg).

These are just the compile failures :) There's also https://github.com/WebAssembly/waterfall/pull/316/files#diff-a6006086cef2fc4f685647d1fe006766

Jan 22 2018, 3:40 PM

Jan 19 2018

dschuff committed rL323012: [WebAssembly] Fix MSVC build.
[WebAssembly] Fix MSVC build
Jan 19 2018, 4:05 PM
dschuff accepted D42284: [WebAssembly] MC: Start table at offset 1 rather than 0.
Jan 19 2018, 10:57 AM
dschuff committed rL322971: [WebAssembly] Fix libcall signature lookup.
[WebAssembly] Fix libcall signature lookup
Jan 19 2018, 9:49 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Jan 19 2018, 9:49 AM
dschuff updated the diff for D42271: [WebAssembly] Fix libcall signature lookup.
  • use StringRef::withNullAsEmpty() and clang-format
Jan 19 2018, 8:44 AM
dschuff added inline comments to D42271: [WebAssembly] Fix libcall signature lookup.
Jan 19 2018, 8:44 AM

Jan 18 2018

dschuff added a reviewer for D42271: [WebAssembly] Fix libcall signature lookup: aheejin.
Jan 18 2018, 4:47 PM
dschuff updated the diff for D42271: [WebAssembly] Fix libcall signature lookup.
  • fix build
Jan 18 2018, 4:43 PM
dschuff created D42271: [WebAssembly] Fix libcall signature lookup.
Jan 18 2018, 4:26 PM