Page MenuHomePhabricator

twoh (Taewook Oh)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 4 2016, 3:03 PM (148 w, 6 d)

Recent Activity

Nov 5 2018

twoh committed rL346151: [MergeICmps] Do not perform the transformation if GEP is used outside of block.
[MergeICmps] Do not perform the transformation if GEP is used outside of block
Nov 5 2018, 10:19 AM
twoh closed D54089: [MergeICmps] Do not perform the transformation if GEP is used outside of block.
Nov 5 2018, 10:19 AM
twoh updated the diff for D54089: [MergeICmps] Do not perform the transformation if GEP is used outside of block.

Comments for the test updated. Thanks for the comment!

Nov 5 2018, 10:13 AM
twoh added a comment to D53911: [Orc] make getResponsibilitySetWithLegacyFn behavior match with LegacyJITSymbolResolver::getResponsibilitySet.

Friendly ping. Thanks!

Nov 5 2018, 8:59 AM

Nov 4 2018

twoh created D54089: [MergeICmps] Do not perform the transformation if GEP is used outside of block.
Nov 4 2018, 9:36 PM

Oct 30 2018

twoh added a comment to D53860: [SemaCXX] Don't check base's dtor is accessible.

@rsmith I see. Thank you for the clarification!

Oct 30 2018, 6:48 PM
twoh created D53911: [Orc] make getResponsibilitySetWithLegacyFn behavior match with LegacyJITSymbolResolver::getResponsibilitySet.
Oct 30 2018, 4:45 PM
twoh added a comment to D53860: [SemaCXX] Don't check base's dtor is accessible.

I think the context is Derived here. My understanding of http://wg21.link/p0968r0#2227 (in this patch's context) is that when Derived is aggregate initialized, the destructor for each element of Base is potentially invoked as well.

Oct 30 2018, 12:57 PM

Oct 29 2018

twoh added a comment to D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.

@rsmith @dblaikie Thank you for the comments! It seems that this is not the appropriate way to handle the issue. I'll find different way to resolve the problem.

Oct 29 2018, 11:17 AM

Oct 25 2018

twoh added a comment to D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.

@dblaikie I see. The problem we're experiencing is that with Clang's naming scheme we end up having different function name between the original source and the preprocessed source (as macro expansion changes the column number). This doesn't work well for me if I want to use distcc on top of our caching system, as the caching scheme expects the output to be same as far as the original source has not been changed (regardless of it's compiled directly or first preprocessed then compiled), but the distcc preprocesses the source locally then sends it to the remote build machine (and we do not turn distcc on for all workflow). I wonder if you have any suggestion to resolve this issue? Thanks!

Oct 25 2018, 8:34 AM

Oct 22 2018

twoh added a comment to D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.

@aprantl It is a debug info. If you compile test/CodeGenCXX/debug-info-anonymous.cpp file with clang -g2 -emit-llvm -S , you will find debug metadata something like distinct !DISubprogram(name: "foo<X::(anonymous enum at /home/twoh/llvms/llvm/tools/clang/test/CodeGenCXX/debug-info-anonymous.cpp:9:3)>", linkageName: "_Z3fooIN1XUt_EEiT_" ..., which will eventually be included in .debug_info section.

Oct 22 2018, 10:47 AM

Oct 19 2018

twoh updated the diff for D34796: upporting -f(no)-reorder-functions flag, clang side change.

Remove conflict line.

Oct 19 2018, 8:49 AM
twoh added a comment to D34796: upporting -f(no)-reorder-functions flag, clang side change.

@joerg Sorry but I'm not sure if I understand your question. This doesn't pretend to honor source code order, but makes linker to place "hot" functions under .text.hot section (There's no guarantee of ordering between functions inside .hot.text section) while "cold" functions under .text.unlikely section. This is purely for performance.

Oct 19 2018, 8:48 AM

Oct 18 2018

twoh updated the diff for D34795: Supporting -f(no)-reorder-functions flag, llvm side change.

Rebase. Tests are provided in the clang counterpart (D34796).

Oct 18 2018, 11:40 PM
twoh updated the diff for D34796: upporting -f(no)-reorder-functions flag, clang side change.

Rebase. Sorry I somehow missed the recent comments. I addresses @davidxl's comment on documentation. Thanks!

Oct 18 2018, 11:39 PM

Sep 19 2018

twoh added a comment to D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.

ping!

Sep 19 2018, 8:52 AM

Sep 7 2018

twoh updated the diff for D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.

Addressing comments from @echristo. Reverted option name change, and added a test case. Sorry I haven't work on this code for a while so it took time to invent a test case.

Sep 7 2018, 10:54 PM

Sep 5 2018

twoh added a comment to D37310: [Atomic] Merge alignment information from Decl and from Type when emit atomic expression..

Hello, I observed a case where atomic builtin generates libcall when the corresponding sync builtin generates an atomic instruction (https://bugs.llvm.org/show_bug.cgi?id=38846). It seems that the alignment checking for __atomic builtins (line 759 of this patch) results the difference, and wonder if the check is actually necessary. Could anyone please shed some light on understanding this? Thanks!

Sep 5 2018, 9:24 AM

Aug 16 2018

twoh added a comment to D43690: [ThinLTO] Keep available_externally symbols live.

Hello, I wonder if we need to keep linkonce_odr symbols live here as well. I observe a case that a vtable for template class initiated has linkonce_odr linkage and marked dead here, which results compiler crash at WholeProgramDevirt because the global variable for vtable doesn't have initializer (https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/WholeProgramDevirt.cpp#L676 assumes that GV has the initializer). Thanks!

Aug 16 2018, 12:23 AM

Jul 16 2018

twoh updated the diff for D49386: Make tests unsupported if libatomic cannot be found..

Addressing comments from @Hahnfeld. Thanks!

Jul 16 2018, 11:42 AM
twoh added inline comments to D49386: Make tests unsupported if libatomic cannot be found..
Jul 16 2018, 11:12 AM
twoh created D49386: Make tests unsupported if libatomic cannot be found..
Jul 16 2018, 9:51 AM

Apr 25 2018

twoh committed rL330844: [ICP] Do not attempt type matching for variable length arguments..
[ICP] Do not attempt type matching for variable length arguments.
Apr 25 2018, 10:22 AM
twoh closed D46026: [ICP] Do not attempt type matching for variable length arguments..
Apr 25 2018, 10:22 AM
twoh updated the diff for D46026: [ICP] Do not attempt type matching for variable length arguments..

Addressing comment from @mssimpso. Thanks!

Apr 25 2018, 12:39 AM

Apr 24 2018

twoh added a comment to D45527: [OpenMP] Fix affinity API for KMP_AFFINITY=none|compact|scatter.

Hello @jlpeyton @tlwilmar, is there a chance that this breaks runtime/test/ompt/misc/api_calls_from_other_thread.cpp test? ompt_get_num_places() is expected to return 0 from the test but with this patch it returns 1, and I think it is because kmp_affinity_num_masks is set to 1 from kmp_create_affinity_none_places().

Apr 24 2018, 11:41 PM · Restricted Project
twoh created D46026: [ICP] Do not attempt type matching for variable length arguments..
Apr 24 2018, 12:16 PM

Apr 4 2018

twoh committed rL329250: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.
[CallSiteSplitting] Do not perform callsite splitting inside landing pad
Apr 4 2018, 9:20 PM
twoh closed D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.
Apr 4 2018, 9:20 PM
twoh added inline comments to D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.
Apr 4 2018, 9:17 PM
twoh added inline comments to D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.
Apr 4 2018, 12:29 AM
twoh updated the diff for D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.

Addressing comments from @fhahn and @junbuml's comments. Thank you!

Apr 4 2018, 12:25 AM

Apr 3 2018

twoh retitled D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad from Do not perform callsite splitting inside landing pad to [CallSiteSplitting] Do not perform callsite splitting inside landing pad.
Apr 3 2018, 8:13 AM

Apr 2 2018

twoh updated the diff for D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.

Thanks @fhahn for the comments! I added comments about why we don't perform splitting for the callsites inside the landing pad. Please let me know if you think it is too verbose.

Apr 2 2018, 10:45 PM
twoh updated the diff for D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.

Addressing @junbuml's comments. Thanks!

Apr 2 2018, 9:59 PM

Mar 31 2018

twoh created D45130: [CallSiteSplitting] Do not perform callsite splitting inside landing pad.
Mar 31 2018, 3:20 PM

Mar 12 2018

twoh committed rL327358: [ThinLTO] Add funtions in callees metadata to CallGraphEdges.
[ThinLTO] Add funtions in callees metadata to CallGraphEdges
Mar 12 2018, 9:29 PM
twoh closed D44399: [ThinLTO] Add funtions in callees metadata to CallGraphEdges.
Mar 12 2018, 9:29 PM
twoh updated the diff for D44399: [ThinLTO] Add funtions in callees metadata to CallGraphEdges.

Update test to check f2.llvm.0 as well.

Mar 12 2018, 4:08 PM
twoh added a comment to D44399: [ThinLTO] Add funtions in callees metadata to CallGraphEdges.

@tejohnson Thanks for the clarification. Regarding hotness, I'm not sure if providing "some" hotness is better than leaving it as unknown if profile data is not provided (If profile data is given, as you said, VP metadata will be attached to the callsite). I'm afraid that synthesized hotness may confuse optimizers, but please let me know if you have different idea.

Mar 12 2018, 4:07 PM
twoh added a comment to D44399: [ThinLTO] Add funtions in callees metadata to CallGraphEdges.

@tejohnson I think your right. What I meant was that when the metadata is imported to bar.o, it references f1 and f2 by their promoted names, which makes the declarations with the promoted names to be added. Did I get it right, or still miss something?

Mar 12 2018, 3:01 PM
twoh created D44399: [ThinLTO] Add funtions in callees metadata to CallGraphEdges.
Mar 12 2018, 12:16 PM

Dec 6 2017

twoh committed rL319904: Stringizing raw string literals containing newline.
Stringizing raw string literals containing newline
Dec 6 2017, 9:01 AM
twoh committed rC319904: Stringizing raw string literals containing newline.
Stringizing raw string literals containing newline
Dec 6 2017, 9:01 AM
twoh closed D39279: Stringizing raw string literals containing newline by committing rC319904: Stringizing raw string literals containing newline.
Dec 6 2017, 9:01 AM

Dec 5 2017

twoh updated the diff for D39279: Stringizing raw string literals containing newline.

clang-format

Dec 5 2017, 10:06 AM
twoh updated the diff for D39279: Stringizing raw string literals containing newline.

@jkorous-apple Got it. I agree that it would be better to move the comments to the header. Will land it soon. Thanks!

Dec 5 2017, 9:48 AM

Dec 4 2017

twoh updated the diff for D39279: Stringizing raw string literals containing newline.

Thanks @jkorous-apple for the comment. I think your suggestion is a more
precise description for the implementation, and adjusted the comments
accordingly.

Dec 4 2017, 10:35 AM

Nov 29 2017

twoh added a comment to D39279: Stringizing raw string literals containing newline.

@jkorous-apple Thanks for the comments! Yeah, I was thinking of O(lenght_of_string) approach, but considering the complicatedness of the implementation (I guess the real implementation would be a bit more complex than your pseudo implementation to handle quote and '\n\r' '\r\n' cases) I decided to stay with O(length_of_string * number_of_endlines_in_string) but optimizing the number of move operations.

Nov 29 2017, 10:29 AM

Nov 27 2017

twoh updated the diff for D39279: Stringizing raw string literals containing newline.

Thanks @jkorous-apple for your comments. I modified the type for the variables and replaced unnecessary inserts and erases with updates.

Nov 27 2017, 10:08 PM

Nov 20 2017

twoh updated the diff for D39279: Stringizing raw string literals containing newline.

Addressing @vsapsai's comments. Thank you for the suggestion! Added test case actually finds an off-by-one error in the original patch. I improved the comments as well.

Nov 20 2017, 2:35 PM

Nov 1 2017

twoh added a comment to D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.

Ping. Thanks!

Nov 1 2017, 1:29 PM
twoh added a comment to D39279: Stringizing raw string literals containing newline.

Ping. Thanks!

Nov 1 2017, 1:28 PM

Oct 25 2017

twoh updated the diff for D39279: Stringizing raw string literals containing newline.

clang-format

Oct 25 2017, 12:01 AM

Oct 24 2017

twoh created D39279: Stringizing raw string literals containing newline.
Oct 24 2017, 11:58 PM

Sep 29 2017

twoh added a comment to D38199: Make test supported only when armv7s target is available..

Ping. Thanks!

Sep 29 2017, 8:57 AM

Sep 27 2017

twoh added a comment to D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.

Ping, Thanks!

Sep 27 2017, 11:30 AM

Sep 25 2017

twoh created D38199: Make test supported only when armv7s target is available..
Sep 25 2017, 5:02 AM

Sep 19 2017

twoh updated the summary of D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.
Sep 19 2017, 6:56 PM
twoh created D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used.
Sep 19 2017, 4:31 PM

Aug 28 2017

twoh committed rL311906: Create PHI node for the return value only when the return value has uses..
Create PHI node for the return value only when the return value has uses.
Aug 28 2017, 11:58 AM
twoh closed D37176: Create PHI node for the return value only when the return value has uses. by committing rL311906: Create PHI node for the return value only when the return value has uses..
Aug 28 2017, 11:58 AM
twoh updated the diff for D37176: Create PHI node for the return value only when the return value has uses..

Addressing @davidxl and @xur's comment. Thanks!

Aug 28 2017, 10:43 AM
twoh updated the diff for D37176: Create PHI node for the return value only when the return value has uses..

Fix a test.

Aug 28 2017, 8:52 AM

Aug 25 2017

twoh updated the summary of D37176: Create PHI node for the return value only when the return value has uses..
Aug 25 2017, 10:10 PM
twoh created D37176: Create PHI node for the return value only when the return value has uses..
Aug 25 2017, 9:12 PM

Aug 16 2017

twoh committed rL311037: Use the file name from linemarker for debug info if an input is preprocessed….
Use the file name from linemarker for debug info if an input is preprocessed…
Aug 16 2017, 12:37 PM
twoh closed D36474: Use the file name from linemarker for debug info if an input is preprocessed source. by committing rL311037: Use the file name from linemarker for debug info if an input is preprocessed….
Aug 16 2017, 12:37 PM
twoh added a comment to D34796: upporting -f(no)-reorder-functions flag, clang side change.

Friendly ping. @davidxl, I think there's no harm to make clang consistent with gcc for compiler options, and I wonder if you have any concerns that I may miss. Thanks!

Aug 16 2017, 10:42 AM
twoh added a comment to D36474: Use the file name from linemarker for debug info if an input is preprocessed source..

Ping. Thanks!

Aug 16 2017, 10:38 AM

Aug 10 2017

twoh committed rL310642: Make .file directive to have basename only.
Make .file directive to have basename only
Aug 10 2017, 11:18 AM
twoh closed D36018: Make .file directive to have basename only by committing rL310642: Make .file directive to have basename only.
Aug 10 2017, 11:18 AM

Aug 8 2017

twoh updated the diff for D36474: Use the file name from linemarker for debug info if an input is preprocessed source..

Addressing dblaikie's comments. Thanks!

Aug 8 2017, 11:10 AM
twoh updated the diff for D36474: Use the file name from linemarker for debug info if an input is preprocessed source..

Relocate.

Aug 8 2017, 10:39 AM
twoh created D36474: Use the file name from linemarker for debug info if an input is preprocessed source..
Aug 8 2017, 10:37 AM

Aug 7 2017

twoh added a comment to D36018: Make .file directive to have basename only.

Ping. Thanks!

Aug 7 2017, 9:35 AM

Aug 5 2017

twoh added a comment to D34796: upporting -f(no)-reorder-functions flag, clang side change.

I think it is generally good to match what GCC does to not to confuse people.

Aug 5 2017, 11:47 AM

Aug 3 2017

twoh committed rL309985: Move unit test to the proper location.
Move unit test to the proper location
Aug 3 2017, 2:08 PM
twoh closed D36282: Move unit test to the proper location by committing rL309985: Move unit test to the proper location.
Aug 3 2017, 2:08 PM
twoh created D36282: Move unit test to the proper location.
Aug 3 2017, 1:13 PM

Jul 31 2017

twoh updated the diff for D34796: upporting -f(no)-reorder-functions flag, clang side change.

Update documentation. Please let me know if I need to update other documents as well. Thanks!

Jul 31 2017, 12:01 PM
twoh added a comment to D34795: Supporting -f(no)-reorder-functions flag, llvm side change.

@davidxl I think it is theoretically possible, if the if branch is not taken on line 294. Did I miss something? Thanks!

Jul 31 2017, 9:55 AM
twoh added a comment to D34795: Supporting -f(no)-reorder-functions flag, llvm side change.

Ping. Thanks!

Jul 31 2017, 8:51 AM
twoh added a comment to D34796: upporting -f(no)-reorder-functions flag, clang side change.

Ping. Thanks!

Jul 31 2017, 8:51 AM

Jul 28 2017

twoh updated the diff for D36018: Make .file directive to have basename only.

Delete unnecessary line from the test.

Jul 28 2017, 2:11 PM
twoh created D36018: Make .file directive to have basename only.
Jul 28 2017, 2:03 PM

Jun 29 2017

twoh committed rL306758: Remove redundant copy in recurrences.
Remove redundant copy in recurrences
Jun 29 2017, 4:11 PM
twoh closed D31821: Remove redundant copy in recurrences by committing rL306758: Remove redundant copy in recurrences.
Jun 29 2017, 4:11 PM
twoh updated the diff for D31821: Remove redundant copy in recurrences.

@wmi Good call! I fixed the code per your suggestion. Thanks!

Jun 29 2017, 1:31 PM
twoh updated the diff for D31821: Remove redundant copy in recurrences.

minor fix.

Jun 29 2017, 11:04 AM
twoh updated the diff for D31821: Remove redundant copy in recurrences.

Addressing comments from @wmi. Thank you for the suggestion!

Jun 29 2017, 11:03 AM

Jun 28 2017

twoh added a comment to D34795: Supporting -f(no)-reorder-functions flag, llvm side change.

https://reviews.llvm.org/D34796 is clang side change.

Jun 28 2017, 6:12 PM
twoh created D34796: upporting -f(no)-reorder-functions flag, clang side change.
Jun 28 2017, 6:10 PM
twoh created D34795: Supporting -f(no)-reorder-functions flag, llvm side change.
Jun 28 2017, 6:05 PM
twoh added a comment to D31821: Remove redundant copy in recurrences.

Ping. Thanks!

Jun 28 2017, 11:15 AM

Jun 25 2017

twoh added a comment to D34373: [LV] Optimize for size when vectorizing loops with tiny trip count.

Hello @Ayal, can you please update the comments per @hfinkel's request so that I can accept the patch? Thanks!

Jun 25 2017, 2:58 PM

Jun 20 2017

twoh updated the diff for D31821: Remove redundant copy in recurrences.

Addressing @wmi's concern by limiting the targets to the recurrence cycles that only the last instruction of the recurrence (that feeds the PHI instruction) can have uses outside of the recurrence. This is not an ideal solution yet, and more fundamental solution (such as having recurrence optimization as a separate pass and/or using live range analysis for it) should follow. But still I think it is worth to have it here.

Jun 20 2017, 1:44 PM
twoh added a comment to D34373: [LV] Optimize for size when vectorizing loops with tiny trip count.
In D34373#785485, @twoh wrote:
In D34373#784975, @twoh wrote:

I think this is a right approach, but concerned that the experimental results I shared on D32451 show that it is generally better to not to vectorize the low trip count loops. @Ayal, I wonder if you have any results that this patch actually improves the performance. Thanks!

I know that we're currently missing opportunities for large vectorizable loops with low (static) trip counts. Smaller inner loops are also good candidates for unpredicated vectorization, but we may need to be a bit careful because of modeling inaccuracies and phase-ordering effects (e.g. if we don't vectorize a loop, then we'll end up unrolling it when the unroller runs).

Got it. My concern was for small single-level loops with low trip counts, as I observe them pretty frequently. I have no objection accepting this patch and improve the cost estimator separately.

Do you mean that you see such loops frequently with dynamically-small trip counts, or with static trip counts? I assume the small loops with (small) static trip counts will generally be unrolled.

Jun 20 2017, 11:11 AM
twoh added a comment to D34373: [LV] Optimize for size when vectorizing loops with tiny trip count.
In D34373#784975, @twoh wrote:

I think this is a right approach, but concerned that the experimental results I shared on D32451 show that it is generally better to not to vectorize the low trip count loops. @Ayal, I wonder if you have any results that this patch actually improves the performance. Thanks!

I know that we're currently missing opportunities for large vectorizable loops with low (static) trip counts. Smaller inner loops are also good candidates for unpredicated vectorization, but we may need to be a bit careful because of modeling inaccuracies and phase-ordering effects (e.g. if we don't vectorize a loop, then we'll end up unrolling it when the unroller runs).

Jun 20 2017, 9:17 AM

Jun 19 2017

twoh added a comment to D34373: [LV] Optimize for size when vectorizing loops with tiny trip count.

I think this is a right approach, but concerned that the experimental results I shared on D32451 show that it is generally better to not to vectorize the low trip count loops. @Ayal, I wonder if you have any results that this patch actually improves the performance. Thanks!

Jun 19 2017, 10:13 PM