Page MenuHomePhabricator

jgravelle-google (Jacob Gravelle)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 31 2016, 4:27 PM (145 w, 6 d)

Recent Activity

Feb 25 2019

jgravelle-google accepted D58656: [WebAssembly] Properly align fp128 arguments in outgoing varargs arguments.

Nice, lgtm

Feb 25 2019, 5:25 PM · Restricted Project

Jan 28 2019

jgravelle-google accepted D56938: [WebAssembly] Handle more types of uses in WebAssemblyAddMissingPrototypes.

Lgtm
Seems good and simple, ship it

Jan 28 2019, 4:02 PM

Nov 8 2018

jgravelle-google accepted D54273: [WebAssembly] Fix LowerEmscriptenEHSjLj when there's only longjmp.
Nov 8 2018, 1:59 PM

Oct 3 2018

jgravelle-google added a comment to D49034: [WebAssembly] Move/clone DBG_VALUE during WebAssemblyRegStackify pass.

Forgot to add, need to add __attribute__((used, visibility("default"))) to btPersistentManifold::clearUserCache in that cpp file

Oct 3 2018, 8:19 AM · debug-info

Oct 2 2018

jgravelle-google added a comment to D49034: [WebAssembly] Move/clone DBG_VALUE during WebAssemblyRegStackify pass.

This still breaks two emscripten tests, test_the_bullet and test_poppler
In particular, it creates a wasm module that fails to typecheck, seemingly because of a reordering of instructions. Commenting out MoveDebugValues and CloneDebugValues is needed to avoid this.
The simplest reproduction I can find is:

wasm32-clang++ $EMSCRIPTEN/tests/bullet/src/BulletCollision/NarrowPhaseCollision/btPersistentManifold.cpp \
  -D__EMSCRIPTEN__=1 \
  -I$EMSCRIPTEN/tests/bullet/src/ \
  -nostdlib \
  -Xclang -nostdsysteminc \
  -Xclang -isystem$EMSCRIPTEN/system/include/libc \
  -Xlinker --no-entry \
  -Xlinker --allow-undefined \
  -O2 -g -o test.wasm

where EMSCRIPTEN is the path to emscripten's main directory (containing emcc.py et al). This creates a test.wasm that is invalid, quickly verifiable with wabt's wasm2wat

Oct 2 2018, 2:52 PM · debug-info

Sep 28 2018

jgravelle-google accepted D49208: [WebAssembly] Update WebAssemblyLowerEmscriptenEHSjLj to support separate compilation..
Sep 28 2018, 3:43 PM

Sep 27 2018

jgravelle-google added a comment to D52580: Refactor WasmSignature and use it for MCSymbolWasm.

Ah, yeah I saw "dschuff planned changes to this revision." in phabricator and wasn't entirely sure what that meant, but didn't let that slow me down

Sep 27 2018, 9:21 AM
jgravelle-google added a comment to D52580: Refactor WasmSignature and use it for MCSymbolWasm.

Generally looks ok to me

Sep 27 2018, 8:51 AM

Sep 25 2018

jgravelle-google added a comment to D49034: [WebAssembly] Move/clone DBG_VALUE during WebAssemblyRegStackify pass.

This seems to have broken some of the emscripten tests (binaryen2.test_poppler and binaryen2.test_the_bullet) that we run on the wasm waterfall:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fclient.wasm.llvm%2Flinux%2F35933%2F%2B%2Frecipes%2Fsteps%2FExecute_emscripten_testsuite__emwasm_%2F0%2Fstdout

Sep 25 2018, 4:14 PM · debug-info

Sep 5 2018

jgravelle-google accepted D51562: [WebAssembly] Fix signature of `main` in FixFunctionBitcasts.
Sep 5 2018, 3:28 PM

Sep 4 2018

jgravelle-google accepted D51662: [WebAssembly] Made assembler only use stack instruction tablegen defs.
Sep 4 2018, 4:42 PM
jgravelle-google added inline comments to D51662: [WebAssembly] Made assembler only use stack instruction tablegen defs.
Sep 4 2018, 4:10 PM

Aug 29 2018

jgravelle-google accepted D51460: [WebAssembly] Be a little more conservative in WebAssemblyFixFunctionBitcasts.

The change feels a bit screwy to me, but I can't think of any alternative ways to factor this that aren't worse.

Aug 29 2018, 4:16 PM

Aug 27 2018

jgravelle-google added inline comments to D51320: [WebAssembly] Made disassembler only use stack instructions..
Aug 27 2018, 2:48 PM

Aug 20 2018

jgravelle-google added inline comments to D50980: [WebAssembly] Restore __stack_pointer after catch instructions.
Aug 20 2018, 3:17 PM

Aug 16 2018

jgravelle-google added a reviewer for D50859: [WebAssembly] Remove temporary workaround for function bitcasts: sbc100.
Aug 16 2018, 11:34 AM
jgravelle-google created D50859: [WebAssembly] Remove temporary workaround for function bitcasts.
Aug 16 2018, 11:33 AM

Jul 19 2018

jgravelle-google added inline comments to D49517: [WebAssembly] Ensure all bitcasts that would be invalid in wasm are removed by FixFunctionBitcasts pass.
Jul 19 2018, 10:39 AM

Jul 18 2018

jgravelle-google accepted D49391: [WebAssembly] Add missing -mattr=+exception-handling guards.
Jul 18 2018, 11:46 AM

Jul 17 2018

jgravelle-google accepted D49113: [WebAssemlby] Set IsUsedInRegularObj correctly for undefined data symbols.
Jul 17 2018, 12:01 PM

Jul 16 2018

jgravelle-google added a comment to D49160: [WebAssembly] Added default stack-only instruction mode for MC..

I kinda like the use of two separate testing mode flags, to avoid needing to refactor every test to the explicit local form right away.

Jul 16 2018, 12:28 PM

Jul 12 2018

jgravelle-google added inline comments to D49208: [WebAssembly] Update WebAssemblyLowerEmscriptenEHSjLj to support separate compilation..
Jul 12 2018, 10:11 AM

Jun 21 2018

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

Better would be the appropriate subdirectory of test/Transforms.

In theory because the actual crash is in DeadStoreElimination I could put it there?

Jun 21 2018, 12:39 PM
jgravelle-google retitled D48404: Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion from Don't modify LibFuncs in LTO to Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion.
Jun 21 2018, 12:22 PM
jgravelle-google updated the diff for D48404: Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion.
  • Move test, remove triple
Jun 21 2018, 11:46 AM
jgravelle-google added a comment to D48404: Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion.

Probably just IPO then. It's still not WebAssembly specific.

Jun 21 2018, 11:40 AM
jgravelle-google added a comment to D48443: [WebAssembly] Add no-prototype attribute to prototype-less C functions.

Looks very reasonable and straightforward. LGTM in spirit, but I'll wait for someone who knows Clang better.

Jun 21 2018, 10:39 AM

Jun 20 2018

jgravelle-google created D48404: Don't modify LibFuncs in DeadArgumentElimination or ArgumentPromotion.
Jun 20 2018, 4:19 PM
jgravelle-google accepted D48394: [WebAssembly] Error on mismatched function signature in final output .
Jun 20 2018, 4:06 PM
jgravelle-google accepted D48371: [WebAssembly] Update know failures for the wasm waterfall.
Jun 20 2018, 8:21 AM

Jun 14 2018

jgravelle-google added a comment to D48183: [WebAssembly] Modified tablegen defs to have 2 parallel instuction sets..

Overall looks ok

Jun 14 2018, 2:30 PM

Apr 27 2018

jgravelle-google accepted D46161: [DAGCombiner] Fix a case of 1 in non-splat vector pow2 divisor.
Apr 27 2018, 2:43 PM

Apr 24 2018

jgravelle-google added inline comments to D45796: [WebAssembly] Support imports from custom module names.
Apr 24 2018, 12:22 PM · Restricted Project

Apr 19 2018

jgravelle-google accepted D45825: [WebAssembly] Fix bug where reloc addends were written as unsigned.
Apr 19 2018, 3:28 PM

Apr 4 2018

jgravelle-google accepted D45280: WebAssembly: Never write more than 32-bits for WebAssembly::OPERAND_OFFSET32.
Apr 4 2018, 12:38 PM
jgravelle-google accepted D45102: Fix typo in comment -fmath-errno=0 -> -fno-math-errno.
Apr 4 2018, 10:39 AM

Mar 30 2018

jgravelle-google created D45103: [WebAssembly] Register wasm passes with the PassRegistry.
Mar 30 2018, 12:25 PM
jgravelle-google accepted D45064: [WebAssembly] Refactor tablegen for store instructions (NFC).

Nice!

Mar 30 2018, 8:13 AM

Mar 27 2018

jgravelle-google accepted D44930: [WebAssembly] Add exception and selector intrinsics.
Mar 27 2018, 7:46 AM

Mar 15 2018

jgravelle-google added inline comments to D44329: [WebAssembly] Added initial AsmParser implementation..
Mar 15 2018, 3:27 PM

Mar 9 2018

jgravelle-google added a comment to D44329: [WebAssembly] Added initial AsmParser implementation..

WIP review, read up through WebAssemblyAsmParser::ParseInstruction

Mar 9 2018, 4:41 PM

Mar 2 2018

jgravelle-google added a comment to D43675: [WebAssembly] Rename imported/exported memory symbol to __linear_memory.

I don't think we want to add a commandline flag for this. Better to pick a name and stick with it. If that's going to be "memory" - then fine. But there's no-one asking for it to be customised.

I agree, and I think "no one is asking for this" is an important point. I think if people ask for it in the future it will be something we can do, which is why I'm biased towards leaving this as-is for now.

Mar 2 2018, 10:02 AM

Mar 1 2018

jgravelle-google added a comment to D43675: [WebAssembly] Rename imported/exported memory symbol to __linear_memory.

Additional thoughts of mine:

Mar 1 2018, 2:19 PM

Feb 23 2018

jgravelle-google accepted D43683: [WebAssembly] Add exception handling option and feature.
Feb 23 2018, 4:01 PM

Feb 16 2018

jgravelle-google added a comment to D43399: [WebAssembly] Fix bug is function signature checking.

Function-bitcast stuff looks good to me

Feb 16 2018, 11:30 AM

Feb 12 2018

jgravelle-google added inline comments to D43205: [WebAssembly] Fix casting MCSymbol to MCSymbolWasm on ELF.
Feb 12 2018, 12:27 PM
jgravelle-google updated the diff for D43205: [WebAssembly] Fix casting MCSymbol to MCSymbolWasm on ELF.
  • Gate MCSymbolWasm on BinFormatWasm
Feb 12 2018, 12:24 PM
jgravelle-google created D43205: [WebAssembly] Fix casting MCSymbol to MCSymbolWasm on ELF.
Feb 12 2018, 12:12 PM

Oct 31 2017

jgravelle-google added inline comments to D39355: [dsymutil] Implement the --threads option.
Oct 31 2017, 4:06 PM

Oct 9 2017

jgravelle-google updated the diff for D38640: [WebAssembly] Narrow the scope of WebAssemblyFixFunctionBitcasts.
  • Use CallSites, properly test exceptions
Oct 9 2017, 4:15 PM
jgravelle-google updated the diff for D38640: [WebAssembly] Narrow the scope of WebAssemblyFixFunctionBitcasts.
  • Add invoke test
Oct 9 2017, 10:14 AM

Oct 6 2017

jgravelle-google added a comment to D38640: [WebAssembly] Narrow the scope of WebAssemblyFixFunctionBitcasts.

Is the assumption here that the eventual caller will cast the argument back to the original type?

Pretty much. In C, that is explicitly defined to be well-defined, and otherwise it's undefined behavior. Native targets ignore missing args and/or read junk bits from the argument registers, wasm traps at runtime, or with this pass we synthesize/ignore args.

Oct 6 2017, 4:56 PM
jgravelle-google created D38640: [WebAssembly] Narrow the scope of WebAssemblyFixFunctionBitcasts.
Oct 6 2017, 11:36 AM

Oct 4 2017

jgravelle-google accepted D38523: [WebAssembly] Add the rest of the atomic loads.

Nice

Oct 4 2017, 11:26 AM

Sep 15 2017

jgravelle-google accepted D37931: Remove __builtin_wasm_rethrow builtin.
Sep 15 2017, 3:00 PM
jgravelle-google accepted D37916: [WebAssembly] MC: Fix crash in getProvitionalValue on weak references.

Looks good to me (take that with a grain of salt, but this seems straightforward?)

Sep 15 2017, 12:11 PM

Sep 12 2017

jgravelle-google accepted D37603: [WebAssembly] Add sign extend instructions from atomics proposal.
Sep 12 2017, 9:54 AM

Aug 31 2017

jgravelle-google accepted D37345: [WebAssembly] Refactor load ISel tablegen patterns into classes.
Aug 31 2017, 2:48 PM
jgravelle-google added a comment to D37345: [WebAssembly] Refactor load ISel tablegen patterns into classes.

As an overall comment, this is awesome. Big fan of reducing syntactic noise.

Aug 31 2017, 1:29 PM

Aug 30 2017

jgravelle-google accepted D37300: [WebAssembly] Add target feature for atomics.

Nits aside lgtm

Aug 30 2017, 9:39 AM

Aug 24 2017

jgravelle-google retitled D37073: [WebAssembly] FastISel : Bail to SelectionDAG for constexpr calls from [WebAssembly] FastISel : Lower constant calls as direct calls to [WebAssembly] FastISel : Bail to SelectionDAG for constexpr calls.
Aug 24 2017, 11:35 AM
jgravelle-google updated the diff for D37073: [WebAssembly] FastISel : Bail to SelectionDAG for constexpr calls.
  • Add basic blocks to call_constexpr test
Aug 24 2017, 11:32 AM
jgravelle-google updated the diff for D37073: [WebAssembly] FastISel : Bail to SelectionDAG for constexpr calls.
  • Test more constexpr instructions, make test name more explicit
Aug 24 2017, 10:41 AM

Aug 23 2017

jgravelle-google created D37073: [WebAssembly] FastISel : Bail to SelectionDAG for constexpr calls.
Aug 23 2017, 9:45 AM

Aug 22 2017

jgravelle-google updated the diff for D36989: [clang-diff] Refactor stop-after command-line flag.
  • Undo refactor, just change name
Aug 22 2017, 10:12 AM
jgravelle-google added a comment to D36989: [clang-diff] Refactor stop-after command-line flag.

If you have more stop-after options it'd probably be simpler to leave it as one field. I'll just rename it to "stop-diff-after" to avoid the name collision.

Aug 22 2017, 10:08 AM

Aug 21 2017

jgravelle-google created D36989: [clang-diff] Refactor stop-after command-line flag.
Aug 21 2017, 4:06 PM

Jul 25 2017

jgravelle-google added a comment to D35852: Quote '?' in llvm-rc test.

I'm not sure off the top of my head if this works the same on Windows' cmd.exe. If someone can check this on a Windows box that would be useful.

Jul 25 2017, 1:16 PM
jgravelle-google created D35852: Quote '?' in llvm-rc test.
Jul 25 2017, 1:16 PM

Jun 21 2017

jgravelle-google added a comment to D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.

@sunfish , ping?

Jun 21 2017, 5:17 PM
jgravelle-google accepted D34477: [WebAssembly] Cleanup WasmObjectWriter.cpp. NFC.

Lots of nice changes

Jun 21 2017, 4:34 PM

Jun 16 2017

jgravelle-google added a comment to D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.

A more precise fix would be to have that code at the bottom of WebAssemblyFastISel::computeAddress return false in the case where there's already a (non-zero) base register.

That's probably worth doing, the problem is that the line that does this problematic folding already checks Addr.getReg() == 0, so it doesn't actually fix the problem.
The real underlying cause is that we think we're folding Add instructions, but we haven't actually checked that the Use is an Add yet. Moving the unscaled add after the check makes that a correct assumption, and everything seems happy about it.

The if (S == 1 && Addr.isRegBase() && Addr.getReg() == 0) case doesn't depend on the use being an Add.

The comment // An unscaled add of a register. Set it as the new base. confused me then, especially because immediately following we are dealing with Adds.
Thinking about it more deeply, this is is basically when we have getelementptr i8, i8* %pointer, i32 %someExpression, and this lowers that to the wasm equivalent of (i32.add %pointer %someExpression)?

Jun 16 2017, 3:18 PM
jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.

Remove unintended files

Jun 16 2017, 3:10 PM
jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
  • Guard against Addr base being set in Alloca case
Jun 16 2017, 3:05 PM
jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
  • Add fast-isel-abort=1 to test
Jun 16 2017, 10:55 AM

Jun 13 2017

jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
  • Re-add check before final setReg in computeAddress
Jun 13 2017, 3:10 PM
jgravelle-google updated subscribers of D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
Jun 13 2017, 3:06 PM
jgravelle-google added inline comments to D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
Jun 13 2017, 2:59 PM
jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
  • Remove unneeded continue
Jun 13 2017, 2:59 PM
jgravelle-google added a comment to D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.

A more precise fix would be to have that code at the bottom of WebAssemblyFastISel::computeAddress return false in the case where there's already a (non-zero) base register.

That's probably worth doing, the problem is that the line that does this problematic folding already checks Addr.getReg() == 0, so it doesn't actually fix the problem.
The real underlying cause is that we think we're folding Add instructions, but we haven't actually checked that the Use is an Add yet. Moving the unscaled add after the check makes that a correct assumption, and everything seems happy about it.
Note that adding the return false made the lit tests pass, and only failed on the wasm waterfall. I added a test case (@store_i8_with_array_alloca_gep) to cover it.
This also only fails with the -elf triple, because we store the stack pointer in memory instead of in a global, making allocas loads instead of get_globals.

Jun 13 2017, 2:36 PM
jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
  • Remove commented out code
Jun 13 2017, 2:20 PM
jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
  • Rebase
Jun 13 2017, 2:19 PM
jgravelle-google updated the diff for D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
  • Reduce nesting in computeAddress, add test case for GEP flattening
Jun 13 2017, 2:18 PM
jgravelle-google accepted D34131: [WebAssembly] Cleanup WebAssemblyWasmObjectWriter.
Jun 13 2017, 10:45 AM

Jun 8 2017

jgravelle-google created D34044: [WebAssembly] WebAssemblyFastISel getelementptr variable index support.
Jun 8 2017, 12:00 PM
jgravelle-google added a comment to D33962: [WebAssembly] MC: Fix value of R_WEBASSEMBLY_TABLE_INDEX relocations.

Looks fine but I don't understand it really. @sunfish ?

Jun 8 2017, 11:49 AM

Jun 6 2017

jgravelle-google accepted D33905: [WebAssembly] Remove unused methods from MCWasmObjectTargetWriter.
Jun 6 2017, 9:32 AM
jgravelle-google added inline comments to D33918: [WebAssembly] MC: Refactor relocation handling.
Jun 6 2017, 9:25 AM

Jun 5 2017

jgravelle-google added a comment to D33918: [WebAssembly] MC: Refactor relocation handling.

Overall looks reasonable to me? Take that for what it's worth.

Jun 5 2017, 4:34 PM

Jun 1 2017

jgravelle-google created D33811: Revert r304117 - WebAssembly object format isn't ready to be the default.
Jun 1 2017, 5:04 PM
jgravelle-google added a comment to D33792: [WebAssembly] Refactor WasmObjectWriter::writeObject.

Lgtm

Jun 1 2017, 3:50 PM
jgravelle-google added a comment to D33803: [WebAssembly] MC: Fix references to undefined externals in data section.

I don't know that much about MC but looks fine to me

Jun 1 2017, 3:03 PM

May 17 2017

jgravelle-google created D33295: [WebAssembly][NFC] Update expected testsuite failures for newly passing tests.
May 17 2017, 12:54 PM

May 1 2017

jgravelle-google abandoned D29736: [WebAssembly] Add target specific overrides for lgamma family functions.
May 1 2017, 9:46 AM

Apr 17 2017

jgravelle-google created D32136: [WebAssembly] Fix WebAssemblyOptimizeReturned after r300367.
Apr 17 2017, 2:02 PM

Feb 15 2017

jgravelle-google added a comment to D29778: Declare lgamma library builtins as never being const.

Thanks, added a comment

Feb 15 2017, 4:58 PM
jgravelle-google updated the diff for D29778: Declare lgamma library builtins as never being const.
  • Add comment to lgamma builtins
Feb 15 2017, 4:54 PM

Feb 9 2017

jgravelle-google created D29778: Declare lgamma library builtins as never being const.
Feb 9 2017, 11:35 AM
jgravelle-google added a comment to D29736: [WebAssembly] Add target specific overrides for lgamma family functions.

https://clang.llvm.org/compatibility.html doesn't mention anything about POSIX, only C11 compliance, so I didn't think Clang in general cared about POSIX.
That being said I can definitely agree that Clang shouldn't preclude POSIX. I'll open a more-general diff.

Feb 9 2017, 9:53 AM

Feb 8 2017

jgravelle-google created D29736: [WebAssembly] Add target specific overrides for lgamma family functions.
Feb 8 2017, 4:01 PM