Page MenuHomePhabricator

takuto.ikuta (Takuto Ikuta)
User

Projects

User does not belong to any projects.

User Details

User Since
May 4 2017, 12:17 AM (123 w, 5 d)

Recent Activity

Apr 17 2019

takuto.ikuta added inline comments to D60095: [LLD][COFF] Early load PDB type server files.
Apr 17 2019, 4:42 PM · Restricted Project

Apr 13 2019

takuto.ikuta added a comment to D60609: Use native llvm JSON library for time profiler output.

I think it is true that json library is slower than just naive json output.
But this code run one time in a compile and should have relatively few json objects.
So visible performance effect will be low. Taking stats sounds good and I'm surprised if this patch change clang performance match.

Apr 13 2019, 1:11 AM · Restricted Project

Apr 12 2019

takuto.ikuta accepted D60609: Use native llvm JSON library for time profiler output.

Thank you for follow up.

Apr 12 2019, 6:34 PM · Restricted Project

Apr 8 2019

takuto.ikuta accepted D60404: Improve hashing for time profiler.

LGTM with nit.
But I hope rnk take a look.

Apr 8 2019, 9:16 PM · Restricted Project
takuto.ikuta requested changes to D60404: Improve hashing for time profiler.
Apr 8 2019, 6:18 PM · Restricted Project
takuto.ikuta added inline comments to D60404: Improve hashing for time profiler.
Apr 8 2019, 7:16 AM · Restricted Project

Mar 27 2019

takuto.ikuta added inline comments to D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.
Mar 27 2019, 6:45 AM · Restricted Project, Restricted Project

Mar 26 2019

takuto.ikuta added inline comments to D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.
Mar 26 2019, 8:26 AM · Restricted Project, Restricted Project

Mar 23 2019

takuto.ikuta added inline comments to D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.
Mar 23 2019, 4:13 AM · Restricted Project, Restricted Project

Nov 12 2018

takuto.ikuta updated the diff for D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.

rebase

Nov 12 2018, 8:14 PM
takuto.ikuta added a comment to D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.

Thank you for review!

Nov 12 2018, 7:19 AM
takuto.ikuta updated the diff for D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.

short diag

Nov 12 2018, 7:13 AM
takuto.ikuta retitled D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag from [clang-cl] Do not allow to use both /Zc:dllexportInlines- and /fallback to [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.
Nov 12 2018, 6:36 AM
takuto.ikuta created D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.
Nov 12 2018, 6:30 AM

Nov 10 2018

takuto.ikuta accepted D54370: [codeview] Expose -gcodeview-ghash for global type hashing.

This change makes sense to me.

Nov 10 2018, 7:32 PM
takuto.ikuta accepted D54319: clang-cl: Add documentation for /Zc:dllexportInlines-.

Seems good document!

Nov 10 2018, 7:24 PM

Nov 9 2018

takuto.ikuta added a comment to D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback.

Thank you for review!

Nov 9 2018, 4:59 AM
takuto.ikuta updated the diff for D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback.

warn in GetCommand

Nov 9 2018, 4:57 AM
takuto.ikuta retitled D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback from [clang-cl] Add warning for /Zc:dllexportInlines- with /fallback to [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback.
Nov 9 2018, 1:53 AM
takuto.ikuta updated the diff for D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback.

Inlines

Nov 9 2018, 1:52 AM
takuto.ikuta created D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback.
Nov 9 2018, 1:49 AM

Nov 2 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you! I'll submit this if other people does not have other comments.

Nov 2 2018, 6:46 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Fix check-prefix

Nov 2 2018, 6:43 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you for quick review!

Nov 2 2018, 3:24 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.
Nov 2 2018, 3:24 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

added a few more check

Nov 2 2018, 3:24 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Address comment

Nov 2 2018, 3:14 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

I missed the comment about driver flag test.

Nov 2 2018, 12:51 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Added cl driver flag test

Nov 2 2018, 12:51 AM

Nov 1 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Hans, thank you for review! I addressed all your comment and fixed small behavior.

Nov 1 2018, 10:04 PM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Simplify test and fix some behavior.

Nov 1 2018, 10:02 PM

Oct 31 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you for review!

Oct 31 2018, 11:56 PM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Added option to LangOptions.def

Oct 31 2018, 11:30 PM
takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 31 2018, 7:37 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

export/import explicit template instantiation function

Oct 31 2018, 7:35 AM
takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 31 2018, 3:45 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you for quick fix!

Oct 31 2018, 2:48 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Rebased to take r345699

Oct 31 2018, 2:47 AM
takuto.ikuta accepted D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496).

LGTM, thank you for fix!

Oct 31 2018, 1:35 AM

Oct 30 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

I've been thinking more about your example with static locals in lambda and how this works with regular dllexport.

It got me thinking more about the problem of exposing inline functions from a library. For example:

lib.h:

#ifndef LIB_H
#define LIB_H

int foo();

struct __declspec(dllimport) S {
  int bar() { return foo(); }
};

#endif

lib.cc:

#include "lib.h"

int foo() { return 123; }

main.cc:

#include "lib.h"

int main() {
  S s;
  return s.bar();
}

Here, Clang will not emit the body of S::bar(), because it references the non-dllimport function foo(), and trying to referencing foo() outside the library would result in a link error. This is what the DLLImportFunctionVisitor is used for. For the same reason, MSVC will also not inline dllimport functions.

Now, with /Zc:dllexportInlines-, we no longer mark S::bar() dllimport, and so we do emit it, causing that link error. The same problem happens with -fvisibility-inlines-hidden: substitute the declspec above for __attribute__((visibility("default"))) above and try it:

$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so lib.cc
$ g++ main.cc lib.so
/tmp/cc557J3i.o: In function `S::bar()':
main.cc:(.text._ZN1S3barEv[_ZN1S3barEv]+0xd): undefined reference to `foo()'

So this is a known issue with -fvisibility-inlines-hidden. This doesn't come up often in practice, but when it does the developer needs to deal with it.

Oct 30 2018, 1:58 AM

Oct 29 2018

takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 29 2018, 2:43 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Added explanation comment for added attributes and rebased

Oct 29 2018, 2:43 AM

Oct 19 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

ping? Can I go forward in this way?

Oct 19 2018, 1:06 AM

Oct 16 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Hans, I addressed all your comments.
How do you think about current implementation?

Oct 16 2018, 3:43 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

remove comment out code

Oct 16 2018, 1:38 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Fix linkage for inline function of explicit template instantiation declaration

Oct 16 2018, 1:33 AM

Oct 15 2018

takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Remove unnecessary attr creation

Oct 15 2018, 10:42 PM

Oct 14 2018

takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Export function inside explicit template instantiation definition

Oct 14 2018, 11:54 PM
takuto.ikuta retitled D51340: Add /Zc:DllexportInlines option to clang-cl from Add /Zc:DllexportInlines option to clang-cl to [WIP] Add /Zc:DllexportInlines option to clang-cl.
Oct 14 2018, 10:08 PM

Oct 12 2018

takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 12 2018, 3:29 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Address comment

Oct 12 2018, 3:28 AM

Oct 11 2018

takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 11 2018, 8:04 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you for review! I updated the code.

Oct 11 2018, 3:12 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

address comment

Oct 11 2018, 3:11 AM

Oct 10 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you for review!

Oct 10 2018, 4:27 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

address comment

Oct 10 2018, 4:22 AM

Oct 5 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Ping?

Sorry, I know I'm being slow to look at this :-( I have not forgotten, it's just a lot of other things happening at the same time.

Oct 5 2018, 4:51 AM

Oct 4 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Ping?

Oct 4 2018, 3:01 AM

Oct 1 2018

takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 1 2018, 2:53 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you for review!

Oct 1 2018, 2:45 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Update comment for Sema::ActOnFinishInlineFunctionDef

Oct 1 2018, 2:32 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Extract inline function export check to function

Oct 1 2018, 2:26 AM

Sep 24 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Ping?

This patch reduced obj size largely, and I expect this makes distributed build (like goma) faster by reducing data transferred on network.

I'll try to look at it this week.

Have you confirmed on some Chromium file, e.g. stroke_opacity_custom.cc (go/stroke-opactity-custom) that the number of functions that we codegen decreases as expected? I'd expect this to save a lot of compile time.

Sep 24 2018, 7:51 PM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

This patch reduced obj size largely, and I expect this makes distributed build (like goma) faster by reducing data transferred on network.

Sep 24 2018, 7:20 AM

Sep 21 2018

takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 21 2018, 10:12 PM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 21 2018, 10:10 PM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 21 2018, 10:07 PM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 21 2018, 9:28 PM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 21 2018, 4:50 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Remove unnecessary willHaveBody check condition

Sep 21 2018, 4:50 AM

Sep 19 2018

takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 19 2018, 8:57 PM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

PTAL again.

Sep 19 2018, 4:36 AM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 19 2018, 4:29 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 19 2018, 4:25 AM

Sep 18 2018

takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Current implementation cannot build chrome when pch is enabled.
undefined symbol error happens during linking blink_modules.dll

Sep 18 2018, 7:09 AM

Sep 12 2018

takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 12 2018, 8:29 AM

Sep 11 2018

takuto.ikuta retitled D51340: Add /Zc:DllexportInlines option to clang-cl from Add /Zc:DllexportInlines option to clang-cl to [WIP] Add /Zc:DllexportInlines option to clang-cl.
Sep 11 2018, 7:08 AM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 11 2018, 2:10 AM
takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 11 2018, 2:05 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

I'm trying to handle local static var correctly.

Sep 11 2018, 2:04 AM

Sep 10 2018

takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 10 2018, 2:32 AM

Sep 9 2018

takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 9 2018, 10:30 PM

Sep 8 2018

takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 8 2018, 3:57 PM

Sep 7 2018

takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 7 2018, 3:49 PM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 7 2018, 7:38 AM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 7 2018, 6:12 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Did both your builds use PCH? It'd be interesting to see the difference without PCH too; the effect should be even larger.

Added stats of without PCH build.

The summary should probably reference https://bugs.llvm.org/show_bug.cgi?id=33628 and it needs to mention how it affects dllimport too.

Added to description, thanks!

Okay, after reading through the patch, it seems we're still marking class members dllexport, and then you selectively remove the attribute later. That does feel a little bit backward... Does -fvisibility-inlines-hidden also have the static local problem, or how does that flag handle it?

Ah, maybe I can get performance improvement just support fvisibility-inlines-hidden in clang-cl. Let me try.

Sep 7 2018, 5:53 AM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 7 2018, 5:52 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 7 2018, 3:12 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Did both your builds use PCH? It'd be interesting to see the difference without PCH too; the effect should be even larger.

Sep 7 2018, 1:03 AM

Sep 6 2018

takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 6 2018, 11:23 PM
takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 6 2018, 11:05 PM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.

Make patch closer to Nico's original implementation, but warns local static variable instead of detecting it.
In checkClassLevelDLLAttribute, inline function definition is not fully parsed and I cannot make test passed in other way adding export only to inline function having local static variables.

Sep 6 2018, 10:54 PM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

I found that current ToT with original Nico's patch does not allow to link ui_base.dll

https://github.com/atetubou/llvm-project-20170507/tree/totwin_dbg_1236_nico
gives the following link error
https://pastebin.com/9LVRbRVn

I will do bisection to find when some inline functions are not inlined.

Sep 6 2018, 5:39 AM
takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

I found that current ToT with original Nico's patch does not allow to link ui_base.dll

Sep 6 2018, 5:21 AM

Sep 3 2018

takuto.ikuta added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Sorry, this patch has a problem yet. I'm investigating the condition when function inlining happens.

Sep 3 2018, 8:39 AM
takuto.ikuta added a reviewer for D51340: Add /Zc:DllexportInlines option to clang-cl: rnk.
Sep 3 2018, 1:08 AM
takuto.ikuta added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 3 2018, 1:08 AM
takuto.ikuta updated the summary of D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 3 2018, 1:07 AM
takuto.ikuta updated the diff for D51340: Add /Zc:DllexportInlines option to clang-cl.
Sep 3 2018, 12:36 AM