Page MenuHomePhabricator

aheejin (Heejin Ahn)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 29 2016, 12:33 AM (123 w, 5 d)

Recent Activity

Yesterday

aheejin added inline comments to D54924: [WebAssembly] Check if the section order is correct.
Tue, Dec 11, 9:42 PM
aheejin updated the diff for D54924: [WebAssembly] Check if the section order is correct.
  • Remove unused code
Tue, Dec 11, 9:39 PM
aheejin updated the summary of D54924: [WebAssembly] Check if the section order is correct.
Tue, Dec 11, 8:03 PM
aheejin added a comment to D54924: [WebAssembly] Check if the section order is correct.

Also moved the checker to lib/Object.

Tue, Dec 11, 8:01 PM
aheejin updated the diff for D54924: [WebAssembly] Check if the section order is correct.
  • Add custom section checking
Tue, Dec 11, 7:59 PM

Mon, Dec 10

aheejin added inline comments to D55401: [WebAssembly] Fix assembler parsing of br_table..
Mon, Dec 10, 6:35 PM
aheejin committed rL348818: [WebAssembly] Add '.eventtype' directive support.
[WebAssembly] Add '.eventtype' directive support
Mon, Dec 10, 5:15 PM
aheejin closed D55353: [WebAssembly] Add '.eventtype' directive support.
Mon, Dec 10, 5:15 PM
aheejin added a comment to D55347: [WebAssembly] TargetStreamer cleanup (NFC).

Thank you all for comments!

Mon, Dec 10, 5:00 PM
aheejin committed rL348816: [WebAssembly] TargetStreamer cleanup (NFC).
[WebAssembly] TargetStreamer cleanup (NFC)
Mon, Dec 10, 4:59 PM
aheejin closed D55347: [WebAssembly] TargetStreamer cleanup (NFC).
Mon, Dec 10, 4:59 PM
aheejin added a comment to D55401: [WebAssembly] Fix assembler parsing of br_table..

Oh, and just FYI: the ARM code Github link in your CL description wouldn't work once they make changes to the code. You can take a permalink by clicking three dots in the left side of the code and click "Copy permalink", which will get you a link of the specified line in the latest commit. It looks like this: https://github.com/llvm-mirror/llvm/blob/2cc0a7da876c1d8c32775b0119e1e15aaa759b9e/lib/Target/ARM/ARMInstrInfo.td#L550
Also you can do that for for a block of code to by selecting a block by clicking shift and create a permalink for that, like
https://github.com/llvm-mirror/llvm/blob/2cc0a7da876c1d8c32775b0119e1e15aaa759b9e/lib/Target/ARM/ARMInstrInfo.td#L550-L555

Mon, Dec 10, 4:45 PM
aheejin added a comment to D55401: [WebAssembly] Fix assembler parsing of br_table..
  • Could you add "Fixes PR39384." to the CL/commit description?
  • Please run clang-format (and maybe also clang-tidy) on the patch
Mon, Dec 10, 4:15 PM
aheejin added a comment to D55347: [WebAssembly] TargetStreamer cleanup (NFC).

@dschuff Do you think this look OK?

Mon, Dec 10, 2:47 PM
aheejin added a comment to D55347: [WebAssembly] TargetStreamer cleanup (NFC).

@sbc100 Do you have any concerns on landing this?

Mon, Dec 10, 2:06 PM
aheejin added a comment to D55347: [WebAssembly] TargetStreamer cleanup (NFC).

We have 4 possible paths of emitting code (backend vs assembler, binary vs asm), so my answer for "where should this setType call go" is "wherever it makes those 4 paths most uniform / least fragile".

That said, I'd say having the streamer setting state on symbols feels somewhat weird to me, given that a streamer is mostly a dumb byte/text writer, and not responsible for managing state (symbols are owned by MCContext, outside of the streamer). But again, the above rule is more important to me :) We have a lot of places in the code that are somewhat weird, because we're a nonstandard "CPU", so I don't think that should be of primary concern.

Mon, Dec 10, 11:51 AM

Fri, Dec 7

aheejin committed rLLD348703: [WebAssembly] Add support for the event section.
[WebAssembly] Add support for the event section
Fri, Dec 7, 10:21 PM
aheejin committed rL348703: [WebAssembly] Add support for the event section.
[WebAssembly] Add support for the event section
Fri, Dec 7, 10:21 PM
aheejin closed D54875: [WebAssembly] Add support for the event section.
Fri, Dec 7, 10:20 PM
aheejin committed rL348702: [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
[WebAssembly] Make WasmSymbol's signature usable for events (NFC)
Fri, Dec 7, 10:19 PM
aheejin closed D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
Fri, Dec 7, 10:19 PM
aheejin added inline comments to D55347: [WebAssembly] TargetStreamer cleanup (NFC).
Fri, Dec 7, 2:16 PM
aheejin committed rLLD348652: clang-format LLVM.h (NFC).
clang-format LLVM.h (NFC)
Fri, Dec 7, 1:52 PM
aheejin committed rL348652: clang-format LLVM.h (NFC).
clang-format LLVM.h (NFC)
Fri, Dec 7, 1:51 PM
aheejin closed D55406: clang-format LLVM.h (NFC).
Fri, Dec 7, 1:51 PM
aheejin updated the diff for D48345: [WebAssembly] Fix unwind destination mismatches in CFG stackify (WIP).
  • Add unregisterScope() deleted in rL348647
Fri, Dec 7, 1:50 PM
aheejin committed rL348648: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
[WebAssembly] clang-format/clang-tidy AsmParser (NFC)
Fri, Dec 7, 1:38 PM
aheejin closed D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Fri, Dec 7, 1:38 PM
aheejin added inline comments to rL339967: [WebAssembly] CFG stackify support for exception handling.
Fri, Dec 7, 1:36 PM
aheejin added a comment to rL339967: [WebAssembly] CFG stackify support for exception handling.
Fri, Dec 7, 1:35 PM
aheejin committed rL348647: Delete registerScope function.
Delete registerScope function
Fri, Dec 7, 1:34 PM
aheejin updated the diff for D54875: [WebAssembly] Add support for the event section.
  • Address comments
Fri, Dec 7, 1:13 PM

Thu, Dec 6

aheejin updated the diff for D54875: [WebAssembly] Add support for the event section.

Rebase onto D55406

Thu, Dec 6, 7:55 PM
aheejin added a child revision for D55406: clang-format LLVM.h (NFC): D54875: [WebAssembly] Add support for the event section.
Thu, Dec 6, 7:54 PM
aheejin added a parent revision for D54875: [WebAssembly] Add support for the event section: D55406: clang-format LLVM.h (NFC).
Thu, Dec 6, 7:54 PM
aheejin updated the summary of D55406: clang-format LLVM.h (NFC).
Thu, Dec 6, 7:52 PM
aheejin created D55406: clang-format LLVM.h (NFC).
Thu, Dec 6, 7:49 PM
aheejin added inline comments to D54875: [WebAssembly] Add support for the event section.
Thu, Dec 6, 7:45 PM
aheejin updated the diff for D54875: [WebAssembly] Add support for the event section.
  • Address comments
Thu, Dec 6, 7:45 PM
aheejin updated the diff for D55353: [WebAssembly] Add '.eventtype' directive support.
  • Minor fix
Thu, Dec 6, 6:24 PM
aheejin added a comment to D55353: [WebAssembly] Add '.eventtype' directive support.

Since event types can't have return values (they are always no_return right?) should we split out "parseParamList" and have them look like this?

.eventtype  __cpp_exception i32
.eventtype  some_other_exception i32, i32
Thu, Dec 6, 6:22 PM
aheejin updated the diff for D55353: [WebAssembly] Add '.eventtype' directive support.
  • Print only param list for eventtype
Thu, Dec 6, 6:22 PM
aheejin updated the summary of D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Thu, Dec 6, 4:38 PM
aheejin added inline comments to D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Thu, Dec 6, 4:38 PM
aheejin updated the diff for D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
  • Changed else if to if after returns
Thu, Dec 6, 4:36 PM
aheejin added a comment to D54875: [WebAssembly] Add support for the event section.

Ping

Thu, Dec 6, 4:15 PM
aheejin added a comment to D55347: [WebAssembly] TargetStreamer cleanup (NFC).

There is a presensent for the "emit" functions in the streamer modifying symbols. For example:

void MCELFStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {         
  cast<MCSymbolELF>(Symbol)->setSize(Value);                                     
}

I'm not saying this is the best, but it seems to have be designed or used this way in other targets, so I'd be tempted to use caution when refactoring. I myself don't have enough to background to know if this is a direction we want to move in. @sunfish and @aardappel might have a better idea.

Thu, Dec 6, 3:31 PM
aheejin added inline comments to D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Thu, Dec 6, 1:48 PM

Wed, Dec 5

aheejin added inline comments to D55353: [WebAssembly] Add '.eventtype' directive support.
Wed, Dec 5, 10:54 PM
aheejin added a child revision for D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC): D55353: [WebAssembly] Add '.eventtype' directive support.
Wed, Dec 5, 10:17 PM
aheejin added parent revisions for D55353: [WebAssembly] Add '.eventtype' directive support: D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC), D55347: [WebAssembly] TargetStreamer cleanup (NFC).
Wed, Dec 5, 10:17 PM
aheejin added a child revision for D55347: [WebAssembly] TargetStreamer cleanup (NFC): D55353: [WebAssembly] Add '.eventtype' directive support.
Wed, Dec 5, 10:17 PM
aheejin updated the diff for D55353: [WebAssembly] Add '.eventtype' directive support.
  • clang-format & clang-tidy
Wed, Dec 5, 10:16 PM
aheejin updated the summary of D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Wed, Dec 5, 10:12 PM
aheejin updated the diff for D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
  • remove stray newline
Wed, Dec 5, 10:11 PM
aheejin retitled D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC) from [WebAssembly] clang-format AsmParser (NFC) to [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Wed, Dec 5, 10:09 PM
aheejin updated the diff for D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
  • clang-tidy
Wed, Dec 5, 10:07 PM
aheejin created D55353: [WebAssembly] Add '.eventtype' directive support.
Wed, Dec 5, 9:51 PM
aheejin updated the diff for D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
  • Add some newlines
Wed, Dec 5, 8:00 PM
aheejin updated the summary of D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Wed, Dec 5, 8:00 PM
aheejin updated the diff for D55347: [WebAssembly] TargetStreamer cleanup (NFC).
  • clang-format & clang-tidy
Wed, Dec 5, 7:52 PM
aheejin created D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC).
Wed, Dec 5, 7:45 PM
aheejin updated the diff for D55347: [WebAssembly] TargetStreamer cleanup (NFC).
  • Move more setType to AsmPrinter
Wed, Dec 5, 7:23 PM
aheejin updated the summary of D55347: [WebAssembly] TargetStreamer cleanup (NFC).
Wed, Dec 5, 6:32 PM
aheejin updated the summary of D55347: [WebAssembly] TargetStreamer cleanup (NFC).
Wed, Dec 5, 6:27 PM
aheejin updated the summary of D55347: [WebAssembly] TargetStreamer cleanup (NFC).
Wed, Dec 5, 6:25 PM
aheejin added a reviewer for D55347: [WebAssembly] TargetStreamer cleanup (NFC): dschuff.
Wed, Dec 5, 6:24 PM
aheejin retitled D55347: [WebAssembly] TargetStreamer cleanup (NFC) from [WebAssembly] Make variable names consistent in TargetStreamer (NFC) to [WebAssembly] TargetStreamer cleanup (NFC).
Wed, Dec 5, 6:21 PM
aheejin updated the diff for D55347: [WebAssembly] TargetStreamer cleanup (NFC).

More cleanups

Wed, Dec 5, 6:21 PM
aheejin created D55347: [WebAssembly] TargetStreamer cleanup (NFC).
Wed, Dec 5, 4:53 PM
aheejin committed rL348424: [WebAssembly] Change event section code to 13.
[WebAssembly] Change event section code to 13
Wed, Dec 5, 3:13 PM
aheejin closed D55343: [WebAssembly] Change event section code to 13.
Wed, Dec 5, 3:13 PM
aheejin added a comment to D55343: [WebAssembly] Change event section code to 13.

Reference: https://github.com/WebAssembly/exception-handling/issues/70

Wed, Dec 5, 3:03 PM
aheejin created D55343: [WebAssembly] Change event section code to 13.
Wed, Dec 5, 3:02 PM

Tue, Dec 4

aheejin retitled D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC) from [WebAssembly] Make WasmSymbol's signature usable for events to [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
Tue, Dec 4, 10:24 PM
aheejin added inline comments to D55283: CodeGen: Refactor regallocator command line and target selection.
Tue, Dec 4, 9:22 PM
aheejin updated the diff for D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
  • Delete unused operators
Tue, Dec 4, 9:14 PM
aheejin added inline comments to D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
Tue, Dec 4, 9:08 AM

Mon, Dec 3

aheejin updated the summary of D54875: [WebAssembly] Add support for the event section.
Mon, Dec 3, 8:09 PM
aheejin updated the summary of D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
Mon, Dec 3, 8:08 PM
aheejin added a parent revision for D54875: [WebAssembly] Add support for the event section: D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
Mon, Dec 3, 8:08 PM
aheejin added a child revision for D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC): D54875: [WebAssembly] Add support for the event section.
Mon, Dec 3, 8:08 PM
aheejin created D55247: [WebAssembly] Make WasmSymbol's signature usable for events (NFC).
Mon, Dec 3, 8:04 PM
aheejin updated the diff for D54875: [WebAssembly] Add support for the event section.
  • clang-format
Mon, Dec 3, 8:02 PM
aheejin added a comment to D54875: [WebAssembly] Add support for the event section.

@ruiu I add some comments here and there, but it's hard to explain the concept of wasm event fully in the comments only. Wasm globals seem to not have comments for the same reason.

Mon, Dec 3, 8:00 PM
aheejin added inline comments to D54875: [WebAssembly] Add support for the event section.
Mon, Dec 3, 7:58 PM
aheejin updated the diff for D54875: [WebAssembly] Add support for the event section.
Mon, Dec 3, 7:57 PM
aheejin removed a parent revision for D54875: [WebAssembly] Add support for the event section: D54874: [WebAssembly] Make Signature an optional property for every type.
Mon, Dec 3, 4:21 PM
aheejin removed a child revision for D54874: [WebAssembly] Make Signature an optional property for every type: D54875: [WebAssembly] Add support for the event section.
Mon, Dec 3, 4:21 PM
aheejin abandoned D54874: [WebAssembly] Make Signature an optional property for every type.

Abandoning because we decided to drop D54873.

Mon, Dec 3, 3:34 PM
aheejin abandoned D54873: [WebAssembly] Make signature index not a part of EventType.

No, I don't feel strong about this either. I just wanted to say this CL does not lose any sigindex info both in import and non-import cases, because you sounded like otherwise.

Mon, Dec 3, 3:33 PM
aheejin added a comment to D54873: [WebAssembly] Make signature index not a part of EventType.

Yes in that case it's represented like sigindex of functions.

- Type:            FUNCTION                                                    
  FunctionTypes:   [ 0, 0 ]                                                    
- Type:            EVENT                                                       
  EventTypes:      [ 1 ]                   // here                                        
  Events:                                                 
    - Index:           0                                                       
      Attribute:       0
Mon, Dec 3, 2:58 PM
aheejin added a comment to D54873: [WebAssembly] Make signature index not a part of EventType.

And why do we need this for specifying events themselves and for importing events?

I mean event imports need to specify their "type" and non-imported events also need to specify their "type". And in both cases the type needs to include the signature. So I think we should have a unified "eventType" that includes the signature index.

Mon, Dec 3, 2:50 PM
aheejin added inline comments to D55149: [WebAssembly] Enforce assembler emits to streamer in order..
Mon, Dec 3, 5:57 AM

Fri, Nov 30

aheejin added a comment to D55149: [WebAssembly] Enforce assembler emits to streamer in order..

Style nits

Fri, Nov 30, 8:17 PM
aheejin added a comment to D54873: [WebAssembly] Make signature index not a part of EventType.

Ping. So the decision points are

  1. Whether SigIndex should be in or out of WasmEventType?
    1. If SigIndex should be out of WasmEventType, should there even be WasmEventType anymore? Because then there's only Attribute left.
  2. It seems to be people don't like the term Attribute. Should we change it to Kind or something? We can change the spec text, because it's not set in stone or anything.

Seems like we should keep the two member WasmEventType. We need this for specifying the events themselves, and also for importing events. It seems like this matches WasmGlobalType,

I'm not really sure what this means. Do you mean we should keep SigIndex as a part of WasmEventType and drop this CL? And why do we need this for specifying events themselves and for importing events? I don't think I understand what 'this' is actually. And why would it match WasmGlobalType? And do you also suggest, when we get to add multi-value globals later, we should insert SigIndex as a part of WasmGlobalType too?

Fri, Nov 30, 2:55 PM
aheejin added a comment to D54873: [WebAssembly] Make signature index not a part of EventType.

Ping. So the decision points are

  1. Whether SigIndex should be in or out of WasmEventType?
    1. If SigIndex should be out of WasmEventType, should there even be WasmEventType anymore? Because then there's only `Attribute' left.
  2. It seems to be people don't like the term Attribute. Should we change it to Kind or something? We can change the spec text, because it's not set in stone or anything.
Fri, Nov 30, 2:37 PM

Wed, Nov 28

aheejin added inline comments to D54411: [Codegen] Merge tail blocks with no successors after block placement.
Wed, Nov 28, 5:25 PM
aheejin added inline comments to D54873: [WebAssembly] Make signature index not a part of EventType.
Wed, Nov 28, 1:46 PM
aheejin added inline comments to D54873: [WebAssembly] Make signature index not a part of EventType.
Wed, Nov 28, 1:07 PM