silvas (Sean Silva)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 20 2012, 11:51 AM (252 w, 5 d)

Recent Activity

Thu, Jun 15

silvas added a comment to D34222: Split Target.cpp into small files..

Thanks, I like this idea. We might want to do something similar for the tests.

Thu, Jun 15, 2:36 PM

Fri, Jun 9

silvas added a comment to D34067: Use symbols with the default version to resolve unversioned symbols..

If it is based on D33680 I presume it fixes https://bugs.llvm.org/show_bug.cgi?id=28414 also; would be good to mention that in the description too.

Fri, Jun 9, 7:18 PM

Apr 24 2017

silvas added inline comments to D32354: [ELF] - Set DF_STATIC_TLS flag for i386 target..
Apr 24 2017, 11:51 AM

Mar 24 2017

silvas added a comment to D31145: [Outliner] Fix compile-time overhead for candidate choice.

Why is it O(n) on average? It is two nested for loops with some bailouts that only affect the asymptotic runtime if candidates overlap with little-omega(1) other candidates. And it needs to be big-omega(sqrt(n)) if the runtime is to be reduced to even O(n^1.5).

Compiling for AArch64, for the SingleSource and MultiSource tests in the test suite the average length of a candidate is 10 MachineInstrs. The maximum length of a candidate is 108, which appears in a test with 11251 MachineInstrs. So far, it appears to be linear time on average, because for programs with a lot of candidates, the max length of a candidate is usually dominated by the number of candidates. I should really prove the bound though. It's a little tricky because it's O(m * n) where m is the maximum length of a candidate.

Hand waving/brain dump: The tricky case is where m = NumInstructions/k for k > 2 and n ~ O(NumInstructions). As k gets larger, m gets smaller. When m = 2, we only have to compare against one other candidate for each candidate, so we're good.

Previously, since we did some overlap pruning in the tree, we knew that as k gets smaller, the possible number of candidates in the program got smaller. Now that I think about it, that's not really true anymore. I guess the point here would be to show that we do the same number of overlap checks as in the tree here. But then we aren't really talking about pruneOverlaps, so ¯\_(ツ)_/¯

Mar 24 2017, 5:27 PM

Mar 21 2017

silvas added a comment to D31199: Add "(compatible with the GNU linker)" to the -version output..

Btw, I think this is one of the clearest proofs that LLD has crossed a big milestone and is now entering "production quality". You don't see this kind of hack in a non-production-quality program.

Mar 21 2017, 12:46 PM
silvas accepted D31199: Add "(compatible with the GNU linker)" to the -version output..

Lgtm with a some suggestions that I don't feel too strongly about.

Mar 21 2017, 12:27 PM
silvas added inline comments to D31145: [Outliner] Fix compile-time overhead for candidate choice.
Mar 21 2017, 2:57 AM

Mar 20 2017

silvas added a comment to D31145: [Outliner] Fix compile-time overhead for candidate choice.

Overlapping candidates are now handled entirely by the pruneOverlaps method that handled anything the suffix tree pruning didn't handle. Since that method is on average O(n), this behaves far better.

Mar 20 2017, 8:52 PM

Mar 6 2017

silvas added a comment to D30544: [lld] - Ignore non-elf files in archives with --whole-archive.
In D30544#692075, @ruiu wrote:

I'd think we should continue handling it as an error. There are a few reasons:

  1. The GNU linkers don't allow that (thank you George for investigating!). That means almost all Unix projects don't add non-object files to .a files, so in practice the current behavior is desirable for most people.
Mar 6 2017, 12:18 AM

Feb 28 2017

silvas committed rL296580: Improve comment..
Improve comment.
Feb 28 2017, 8:55 PM
silvas committed rL296451: Comment on the typical/simple case of VA calculation.
Comment on the typical/simple case of VA calculation
Feb 28 2017, 1:13 AM
silvas committed rL296448: Add a comment..
Add a comment.
Feb 28 2017, 12:44 AM

Feb 27 2017

silvas committed rL296429: Add Triple::thumb to getBitcodeMachineKind.
Add Triple::thumb to getBitcodeMachineKind
Feb 27 2017, 7:12 PM
silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

Nice to see this finally land!

Feb 27 2017, 5:52 PM

Feb 25 2017

silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

btw, down the road you may want to have this pass really know in detail the encoded length of each instruction on x86. There are quite a few *single instructions* that would be beneficial from a code size perspective to outline (if the outlined function is set to have alignment of 1). A quick analysis of an LLD binary (which contains all of LLVM linked in for LTO) shows there is over 5% code size savings just from outlining single instructions (since many x86 instructions encode to be larger than a CALL instruction which is 5 bytes). About half of the benefit (so about 2-3% of the total on this test case) comes from instructions that reference the stack via %rsp (mostly zeroing out stack slots), which could still be outlined if the offset was rewritten.

Feb 25 2017, 2:37 PM
silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

?!? This should be true for most compiler transformations. I don't know how these problems are handled in practice but I doubt they enable compiler optimizations. I don't see why we should start this discussion with this particular review.

Feb 25 2017, 1:20 PM

Feb 24 2017

silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

I'm not sure how heavily we care about this security aspect as a community, but I'm a slightly wary of having this on by default at any optimization level due to this issue.

Feb 24 2017, 1:39 PM
silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

Also, this pass will almost surely introduce timing side-channel attacks into cryptography code (code that would otherwise by "constant time" and needs to be for security).

Feb 24 2017, 1:38 PM
silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

I just tested on SPECCPU2006 (FullLTO) and no assertion failures!

Feb 24 2017, 1:22 PM

Feb 23 2017

silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

FYI this doesn't build on Linux with gcc

Feb 23 2017, 9:52 PM
silvas accepted D30321: Mention FreeBSD ports status and wordsmithing..

LGTM

Feb 23 2017, 9:27 PM
silvas added inline comments to D26872: Outliner: Add MIR-level outlining pass.
Feb 23 2017, 6:45 PM
silvas added a comment to D27900: [ELF] - Keep the source file/line location information separate from the object file location information..
In D27900#683999, @ruiu wrote:

I don't understand why need "note:" labels in so many places. Why don't you just make error messages multi-line? I don't think there's no such rule that you need to put "note:" after every "\n".

Feb 23 2017, 1:04 AM

Feb 22 2017

silvas added a comment to D27900: [ELF] - Keep the source file/line location information separate from the object file location information..

I think this patch currently implements idea of such output.
(or am I missing something ?)

Feb 22 2017, 12:59 AM

Feb 21 2017

silvas added a comment to D27900: [ELF] - Keep the source file/line location information separate from the object file location information..
In D27900#647823, @ruiu wrote:

Did you actually find that the current error message format is useless in some case? In my experience, the current format works pretty well. It prints out enough information to fix an error and not too verbose. If you still want to pursue this change, please split it, so that we can discuss each change instead of as a whole.

Feb 21 2017, 11:57 PM
silvas added a comment to D29871: Use const-ref in range-loop for to avoid copying pairs of std::string.

r295807, thanks! (and sorry for the delay)

Feb 21 2017, 10:55 PM
silvas committed rL295807: Use const-ref in range-loop for to avoid copying pairs of std::string.
Use const-ref in range-loop for to avoid copying pairs of std::string
Feb 21 2017, 10:45 PM
silvas closed D29871: Use const-ref in range-loop for to avoid copying pairs of std::string by committing rL295807: Use const-ref in range-loop for to avoid copying pairs of std::string.
Feb 21 2017, 10:45 PM

Feb 17 2017

silvas accepted D29871: Use const-ref in range-loop for to avoid copying pairs of std::string.

Thanks, this LGTM. Do you need me to commit this for you?

Feb 17 2017, 11:39 PM

Feb 3 2017

silvas added a comment to D29512: [PGO] Directory name stripping in global identifier for static functions.

This change does two things (as you mention in the description):

Feb 3 2017, 8:31 PM

Feb 1 2017

silvas added a reviewer for D29316: Add predicateinfo intrinsic, analysis pass, and basic NewGVN support: chandlerc.

One key issue here is that this analysis mutates the IR, which is rarely done in LLVM, and I think we are trying to remove all cases of doing that (as Chandler puts it in http://llvm.org/devmtg/2014-04/PDFs/Talks/Passes.pdf slide 17 "Forms a new, sub-set IR, which is problematic"). On that point, you probably want to start a discussion on LLVMdev (with a link to this review). I anticipate at least Chandler is going to have something to say about this w.r.t the new PM, so make sure to CC him.

Feb 1 2017, 6:33 PM
silvas accepted D29259: [PGO] make graph view internal options available for all builds.

LGTM.

Feb 1 2017, 5:07 PM
silvas added a comment to D29305: Strip file path from the -o option while creating reproduce.txt.

Thanks for working on this!

Feb 1 2017, 1:45 PM

Jan 23 2017

silvas added a comment to D29035: [Support] Add sys::fs::set_current_path() (aka chdir).

We have a chdir in lib/Basic/VirtualFileSystem.cpp that probably can be moved to using this API in a followup patch.

Jan 23 2017, 11:33 PM
silvas added a comment to D29035: [Support] Add sys::fs::set_current_path() (aka chdir).

We have a chdir in lib/Basic/VirtualFileSystem.cpp that probably can be moved to using this API in a followup patch.

Jan 23 2017, 11:28 PM
silvas added a comment to D29045: [PGO] Add option to dot-dump raw profile counts as computed by profile annotator.

I won't mind adding documentation of the flags. Is there a known place to do so (for internal options)?

I personally wouldn't mind them to be added to https://clang.llvm.org/docs/UsersManual.html#profile-guided-optimization (maybe in a separate subsection). It's generally nicer having these flags documented somewhere than looking at the code, IMHO

Jan 23 2017, 11:25 PM

Jan 21 2017

silvas committed rL292752: [docs] Point to upstream Sphinx install instructions..
[docs] Point to upstream Sphinx install instructions.
Jan 21 2017, 7:58 PM
silvas committed rL292750: [docs] Fix typo in section heading..
[docs] Fix typo in section heading.
Jan 21 2017, 7:40 PM

Jan 20 2017

silvas added a comment to D28951: [ELF] - Linkerscripts: ignore CONSTRUCTORS in output section declaration..

Are you sure that this CONSTRUCTORS is trying to be a linker keyword? The nearby DATA_DATA is actually a macro expanded from http://src.illumos.org/source/xref/linux-master/include/asm-generic/vmlinux.lds.h#205
Is the CONSTRUCTORS still in file after it has been preprocessed? (I'm guessing so since otherwise this patch wouldn't fix the build, but just wanted to be sure).
Remember to keep in mind that these linker scripts all get preprocessed by the C preprocessor, so remember to look at the preprocessed linker script instead of the original source.

Jan 20 2017, 12:39 PM

Jan 17 2017

silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

In case you're wondering, that didn't take that long to reduce to that point. Just have to learn the tools and workflow. We have amazing tools for doing test case reduction and an easily understandable test-suite build system! (mad props to those who made bugpoint; the folks that made test-suite awesome; and LLD's -save-temps; and a bit of -globalopt and -metarenamer to clean things up)

Jan 17 2017, 12:57 AM
silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

I did a quick run on SPEC CPU 2006 with FullLTO and it seems like I ran into 3 different assertion failures across various programs: https://reviews.llvm.org/P7954
There seem to be 3 different assertions getting hit.

Jan 17 2017, 12:42 AM

Jan 16 2017

silvas added a comment to D28626: RFC: Generalize inverted gc dependencies.

Small nit.

Jan 16 2017, 7:27 PM

Jan 14 2017

silvas added a comment to D28627: [PM] Introduce an analysis set used to preserve all analyses over a function's CFG when that CFG is unchanged..

It would be good to build up some verification machinery here at some point in the future.

Jan 14 2017, 8:08 PM
silvas added a comment to D28627: [PM] Introduce an analysis set used to preserve all analyses over a function's CFG when that CFG is unchanged..

I couldn't come up with a good name for this either. I described a design in my log here: https://docs.google.com/document/d/1-nrq2y_hTiZhrsJDmH8TzFjFEeApfccs6wwGyaXjSkg/edit#heading=h.ut7bc0x07mmh

Jan 14 2017, 8:03 PM

Jan 10 2017

silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

Great work!

Jan 10 2017, 4:34 AM

Jan 8 2017

silvas added a comment to D28444: Define sys::path::convert_to_slash..
In D28444#639425, @ruiu wrote:

Sean,

I'm reluctant to change this type from std::string convert_to_slashes(StringRef) to void convert_to_slashes(SmallVectorImpl<char> &) as the former is easier to use than the latter. In some cases, you can avoid string copy with the latter type, but I think that's very important here. I don't want to optimize unless it is needed.

Jan 8 2017, 5:37 PM

Jan 7 2017

silvas accepted D28444: Define sys::path::convert_to_slash..

can you put the declaration/definition closer to sys::path::native? Other than that this LGTM.

Jan 7 2017, 7:55 PM

Dec 29 2016

silvas added a comment to D28156: Add the 'googlemock' component of Google Test to LLVM's unittest libraries..

And FWIW, based on my experience working on the new PM, I do agree that the new PM unit tests are currently harder to understand and debug than they need to be (for reasons that you outlined well in that LLVMdev thread years ago and in the description of this patch). Especially getting more useful error reports out when the tests fail would be fantastic, so I'd really like to see that aspect of gmock! When you post a patch updating some new PM tests to use gmock, definitely include some example error output from when the test fails.

Dec 29 2016, 11:51 AM
silvas added a comment to D28156: Add the 'googlemock' component of Google Test to LLVM's unittest libraries..

I think that #2 is relatively rarely useful, but there *are* cases where
it is useful. Historically, I think misuse of actual mocking as
described in #2 has led to resistance towards this framework. I am
actually sympathetic to this -- mocking can easily be overused. However
I think this is not a significant concern in LLVM. First and foremost,
LLVM has very careful and rare exposure of abstract interfaces or
dependency injection, which are the most prone to abuse with mocks. So
there are few opportunities to abuse them. Second, a large fraction of
LLVM's unittests are testing *generic code* where mocks actually make
tremendous sense. And gmock is well suited to building interfaces that
exercise generic libraries. Finally, I still think we should be willing
to have testing utilities in tree even if they should be used rarely. We
can use code review to help guide the usage here.

Thoughts?

Dec 29 2016, 11:38 AM

Dec 24 2016

silvas added a comment to D28091: Add a class to create a tar archive file..

This is a good idea. Tar is a much more common format that users will be more familiar with. I'm not an expert on Tar files, so I can't review that part of the patch, but hopefully the bots will show if any tar implementations don't like the files we produce.

Dec 24 2016, 6:43 PM

Dec 21 2016

silvas added inline comments to D27900: [ELF] - Keep the source file/line location information separate from the object file location information..
Dec 21 2016, 11:21 AM

Dec 20 2016

silvas accepted D27969: Move GlobPattern class from LLD to llvm/Support..

Nice! Thanks for adding the unittest too!

Dec 20 2016, 2:30 PM
silvas added inline comments to D27900: [ELF] - Keep the source file/line location information separate from the object file location information..
Dec 20 2016, 2:26 PM

Dec 18 2016

silvas added a comment to D27855: try to extend nonnull-ness of arguments from a callsite back to its parent function.

This would be a good argument for Mehdi's suggestion? Ie, this should be in FunctionAttrs.cpp?

Dec 18 2016, 3:56 PM
silvas added a reviewer for D27855: try to extend nonnull-ness of arguments from a callsite back to its parent function: chandlerc.

This transform seems like it is sensitive to the order in which functions are visited when running function passes, which I think we try to avoid. I.e. if the ordering is right, we can propagate across arbitrarily many transitive calls; but if the ordering is wrong (e.g. we visit callers before callees) then we don't be able to propagate as much.

Dec 18 2016, 2:54 PM
silvas added a comment to D27900: [ELF] - Keep the source file/line location information separate from the object file location information..
error: file-dbg2.o: duplicate symbol 'zed'
error: file-dbg2.a(file-dbg2.o): previous definition was here
note: definitions found in conflict-debug.s(.text+0x0) and conflict-debug.s(.text+0x0)
Dec 18 2016, 1:44 PM
silvas accepted D27819: Remove lld/Support/Memory.h..

Thanks for doing this, LGTM with a nit. Overall, it seems that keeping two copies for now is fine, since it is such a small amount of code.

Dec 18 2016, 12:12 AM

Dec 14 2016

silvas committed rL289748: Rename this variable..
Rename this variable.
Dec 14 2016, 5:08 PM
silvas added a comment to D27198: [PM] Introduce the facilities for registering cross-IR-unit dependencies that require deferred invalidation..

A couple nits. Overall, this is looking a lot better. It's awesome that you sat down with Justin to hash this out.

Dec 14 2016, 3:44 PM
silvas added a comment to D27676: [ELF] - Use full object name if source file name exist when reporting errors..
In D27676#622851, @ruiu wrote:

I thought about this for a while. I'd say that having foo.a(foo.o)(foo.c) and foo.o(foo.c) are not correct because they are inconsistent. If the former were foo.a(foo.o(foo.c)), the two are at least consistent, but I guess that's hard to read.

We have a lot of combinations here.

  • Is an object file in an archive file? Yes or no
  • Do we know source filename? Yes or not
  • What do we know about the error location inside of the object file: Line number in the original source code, section name in an object file, or nothing
Dec 14 2016, 3:15 PM
silvas closed D27778: Rename InputSection.cpp:getSymVA to getRelocTargetVA..

Committed in r289733

Dec 14 2016, 2:57 PM
silvas committed rL289733: Rename InputSection.cpp:getSymVA to getRelocTargetVA..
Rename InputSection.cpp:getSymVA to getRelocTargetVA.
Dec 14 2016, 2:56 PM
silvas updated the diff for D27778: Rename InputSection.cpp:getSymVA to getRelocTargetVA..

Update local variable name to be more consistent.

Dec 14 2016, 2:53 PM
silvas added a comment to D27778: Rename InputSection.cpp:getSymVA to getRelocTargetVA..

I am in favour of this change. I think the name is quite explanatory/clear.
Oracle docs (https://docs.oracle.com/cd/E23824_01/html/819-0690/chapter6-54839.html) use the wording "storage unit affected by the reloc", but I prefer target =)

Dec 14 2016, 2:43 PM
silvas retitled D27778: Rename InputSection.cpp:getSymVA to getRelocTargetVA. from to Rename InputSection.cpp:getSymVA to getTargetVA..
Dec 14 2016, 2:28 PM

Dec 13 2016

silvas added a comment to D27676: [ELF] - Use full object name if source file name exist when reporting errors..
In D27676#621721, @ruiu wrote:

I think my concern is that we are going to print out at most three filenames like this

foo.a(foo.o)(foo.c)

and that error string format looks odd. It is indistinguishable from

foo.o(foo.c)

where in this case foo.o is not an archive but an object file. I think we are using too many parentheses.

Dec 13 2016, 8:51 PM

Dec 12 2016

silvas added a comment to D26224: NewGVN.

A couple more small comments as take another pass looking at the patch.

Dec 12 2016, 10:14 PM

Dec 11 2016

silvas added a comment to D27198: [PM] Introduce the facilities for registering cross-IR-unit dependencies that require deferred invalidation..

A couple suggestions to make this patch more understadable.

Dec 11 2016, 1:38 PM

Dec 9 2016

silvas accepted D27152: Merge strings using sharded hash tables..

LGTM with nits.

Dec 9 2016, 6:03 PM
silvas added a comment to D26872: Outliner: Add MIR-level outlining pass.

Some stylistic comments.

Dec 9 2016, 3:59 AM
silvas added a comment to D26224: NewGVN.

Simon has done a pretty good job with the style review. Once his comments are cleaned up it will be easier to provide further comments.

Dec 9 2016, 3:59 AM
silvas added a comment to D26224: NewGVN.

Also, uhm, this needs tests. Maybe you can get away with just adding some more RUN lines to some LegacyGVN tests?

Dec 9 2016, 3:59 AM

Dec 6 2016

silvas added a comment to D27152: Merge strings using sharded hash tables..

When running with just a single core, can we still do a sharded visitation, but instead of doing NumShards passes through all the pieces (skipping ones not in the current shard), only do a single pass? We should be able to get identical layout in that case.

Dec 6 2016, 9:48 PM

Dec 5 2016

silvas added a comment to D27374: [PM] Rename lookupPass to lookUpPass..

if you're renaming this better to rename it lookUpAnalysis.

I left it as "lookUpPass" because a lot of the analysis code talks about "passes", or sometimes "analysis passes".

Should we excise mentions of "pass" from the analysis parts? I can write that patch if you think it's the terminology we want to adopt.

Dec 5 2016, 9:24 PM

Dec 4 2016

silvas added a comment to D27398: Use "equivalence class" instead of "color" to describe the concept in ICF..

LGTM with a couple wording suggestions. Thanks.

Dec 4 2016, 7:01 PM
silvas accepted D27155: Merge strings using concurrent hash map (3rd try!).

LGTM. Thanks for working on this!

Dec 4 2016, 2:24 PM

Dec 2 2016

silvas added a comment to D27373: [PM] Get rid of an unused variable in AnalysisManager::clear(IRUnitT&)..

Please do this kind of cleanup with post-commit review in the future.

Dec 2 2016, 9:44 PM
silvas added a comment to D27374: [PM] Rename lookupPass to lookUpPass..

If you feel strongly about this then go ahead, though if you're renaming this better to rename it lookUpAnalysis.

Dec 2 2016, 9:41 PM
silvas added a comment to D27372: [PM] Consistently use curly braces rather than std::make_pair in AnalysisResults.find()..

This is fine, but you can use post-commit review on this kind of thing.

Dec 2 2016, 9:35 PM
silvas added a comment to D27368: [PM] Make PreservedAnalyses::preserved take its parameter by const ref..

Feel free to commit this kind of cleanup with post-commit review.

Dec 2 2016, 9:35 PM
silvas added a comment to D27369: [PM] Make PassManager's constructor explicit..

This type of cleanup can be done with post commit review btw.

Dec 2 2016, 9:30 PM
silvas added a comment to D27370: [PM] Make AnalysisManager::registerPass take its parameter by universal reference..

This is fine, but it doesn't really matter; this is only called like once for an entire compilation (it's basically a hack to set a pipeline-global closure for a particular analysis; only AA uses it now)

Dec 2 2016, 9:28 PM
silvas accepted D27295: Remove existing file in a separate thread asynchronously..

Overall, I do like the idea of doing this and this LGTM (although let others chime in). One thing to think about is that if it takes this long to delete a file (even on tmpfs which Joerg mentioned), then how long does it take to allocate the memory for a new file? (let alone write it to disk)

Dec 2 2016, 12:09 AM

Dec 1 2016

silvas added a comment to D26960: [docs] Use x86_64 and i386 instead of x86 as arch for triples..

LGTM. Do you need me to commit this for you?

Dec 1 2016, 12:35 AM
silvas added a comment to D27197: [PM] Support invalidation of inner analysis managers from a pass over the outer IR unit..

nice to see this feature finally implemented!

Dec 1 2016, 12:24 AM
silvas accepted D27247: Parallelize ICF to make LLD's ICF really fast..

This LGTM. Thanks for looking so closely at this! It's a very nice speedup!

Dec 1 2016, 12:16 AM
silvas abandoned D27145: Add "RPRED" mechanism for relocation predicates. (5% speedup for `ld.lld -O0`).

Abandoning now that D27156 has landed.

Dec 1 2016, 12:13 AM

Nov 30 2016

silvas added reviewers for D27295: Remove existing file in a separate thread asynchronously.: joerg, emaste.

Why are we unlinking at all? Hopefully we can just resize the file and ideally reuse most of the pages the kernel already has allocated. Doing so loses the atomicity of the replacement, but so does asynchronously unlinking like you do in this patch (although this patch would manifest the non-atomicity in a more benign way).

Nov 30 2016, 11:55 PM
silvas accepted D27205: [PM] Teach the AAManager and AAResults layer (the worst offender for inter-analysis dependencies) to use the new invalidation infrastructure..

This is just wiring up the new machinery from D27198. Looks good.

Nov 30 2016, 11:42 PM
silvas accepted D27198: [PM] Introduce the facilities for registering cross-IR-unit dependencies that require deferred invalidation..

This makes sense given your current trajectory for the new PM. LGTM with a nit.

Nov 30 2016, 11:39 PM
silvas committed rL288314: Add `isRelExprOneOf` helper.
Add `isRelExprOneOf` helper
Nov 30 2016, 9:53 PM
silvas closed D27156: Add `isRelExprOneOf` helper (alternative to D27145). by committing rL288314: Add `isRelExprOneOf` helper.
Nov 30 2016, 9:53 PM
silvas added inline comments to D27155: Merge strings using concurrent hash map (3rd try!).
Nov 30 2016, 9:37 PM
silvas updated the diff for D27156: Add `isRelExprOneOf` helper (alternative to D27145)..

Small fixup (delete some code that I forgot to remove in the last patch).

Nov 30 2016, 12:41 AM
silvas added a comment to D27156: Add `isRelExprOneOf` helper (alternative to D27145)..

the original simple-minded constexpr containing a for-loop

Nov 30 2016, 12:40 AM
silvas updated the diff for D27156: Add `isRelExprOneOf` helper (alternative to D27145)..

Somewhat simpler template version. Let me know what you think. I think the improved clarity at the call site is worth it.

Nov 30 2016, 12:39 AM

Nov 29 2016

silvas added a comment to D27156: Add `isRelExprOneOf` helper (alternative to D27145)..
In D27156#608816, @ruiu wrote:

I agree that at the call side this looks better, but after reviewing the previous one again, I think I still prefer the previous one, because recursive templates are more complicated than the original simple-minded constexpr containing a for-loop. In practice, static_assert'ing on R_END would be enough as a protection.

Nov 29 2016, 11:43 PM
silvas added a comment to D27247: Parallelize ICF to make LLD's ICF really fast..

Nice! The idea of storing current and next colors solves the nondeterminism in a very simple way!

Nov 29 2016, 11:36 PM
silvas added a comment to D27155: Merge strings using concurrent hash map (3rd try!).

Sorry for the delay.

Nov 29 2016, 10:58 PM
silvas added a comment to D27155: Merge strings using concurrent hash map (3rd try!).

I don't think that it makes sense to put this in a file with a generic name like "Concurrent", as this is a quite specialized data structure that depends on specific LLD types. Maybe just call it ConcurrentStringTableBuilder.h?
I'm a bit worried about the layering too, does this class introduce any circular dependencies?

Nov 29 2016, 10:19 PM