brzycki (Brian Rzycki)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 30 2016, 1:50 PM (102 w, 4 d)

Recent Activity

Mar 20 2018

brzycki updated subscribers of D44629: [CloneFunction] Preserve DT in DuplicateInstructionsInSplitBetween..

I'm not a reviewer, but it LGTM. You might want to ask @kuhar to review too.

Mar 20 2018, 8:08 AM

Mar 19 2018

brzycki added inline comments to D40146: [JumpThreading] Preservation of DT and LVI across the pass.
Mar 19 2018, 9:09 AM
brzycki added a comment to D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Could this lead to problems when jump threading erases an edge?

Mar 19 2018, 8:34 AM

Mar 16 2018

brzycki committed rL327713: [JumpThreading] Track unreachable BBs to avoid processing.
[JumpThreading] Track unreachable BBs to avoid processing
Mar 16 2018, 8:16 AM
brzycki closed D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.
Mar 16 2018, 8:16 AM
brzycki updated the diff for D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Comment cleanup and rebase. NFC.

Mar 16 2018, 7:41 AM

Mar 15 2018

brzycki added a comment to D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Thanks guys. I found a few spelling mistakes in the comments that I'm fixing and testing now. I'll upload the diff and push the change shortly if all goes well.

Mar 15 2018, 7:57 AM
brzycki added a comment to D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Ping. @sebpop, @kuhar, or @dberlin any objections to pushing this patch as-is?

Mar 15 2018, 7:38 AM

Mar 13 2018

brzycki committed rL327432: [LazyValueInfo] PR33357 prevent infinite recursion on BinaryOperator.
[LazyValueInfo] PR33357 prevent infinite recursion on BinaryOperator
Mar 13 2018, 11:16 AM
brzycki closed D34135: [JumpThreading] Use DT to avoid processing dead blocks.
Mar 13 2018, 11:16 AM

Mar 12 2018

brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.

I'm a little worried about the duplicated code here. I feel like you are having to patch way too many points in the same function, and it will be easy to miss one in the future if someone changes the logicc slightly.

Is there really no better place to put this/way to refactor it?

If not, i'll approve, but just wanted to check.

Mar 12 2018, 11:00 AM
brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.

Gentle ping. Are there any objections to this patch? Should I open another Differential for this patch in preparation to commiting it?

Mar 12 2018, 7:38 AM

Mar 8 2018

brzycki updated the diff for D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Addressed @kuhar 's comments.

Mar 8 2018, 3:45 PM
brzycki added a comment to D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

@kuhar I attempted to use a ranged iterator for (auto &BB : F) loop and it causes crashes in test-suite.

FAILED: Bitcode/Benchmarks/Halide/bilateral_grid/CMakeFiles/halide_bilateral_grid.dir/driver.cpp.o
/work/brzycki/test-suite/build/tools/timeit --summary Bitcode/Benchmarks/Halide/bilateral_grid/CMakeFiles/halide_bilateral_grid.dir/driver.cpp.o.time /work/brzycki/upstream/install/bin/clang++  -DNDEBUG -I../Bitcode/Benchmarks/Halide/bilateral_grid -IBitcode/Benchmarks/Halide/bilateral_grid -O3   -O3 -DNDEBUG   -w -Werror=date-time -std=c++11 -MD -MT Bitcode/Benchmarks/Halide/bilateral_grid/CMakeFiles/halide_bilateral_grid.dir/driver.cpp.o -MF Bitcode/Benchmarks/Halide/bilateral_grid/CMakeFiles/halide_bilateral_grid.dir/driver.cpp.o.d -o Bitcode/Benchmarks/Halide/bilateral_grid/CMakeFiles/halide_bilateral_grid.dir/driver.cpp.o -c ../Bitcode/Benchmarks/Halide/bilateral_grid/driver.cpp
While deleting: label %lpad142
Mar 8 2018, 10:58 AM

Mar 7 2018

brzycki added a comment to D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Hi @kuhar , thanks for the review. As a general comment I tried to minimize changes to JumpThreading to remove removeUnreachableBlocks.

Mar 7 2018, 2:47 PM
brzycki updated the diff for D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Renamed InvalidBlocks to UnreachableBlocks, minor tweaks to comments.

Mar 7 2018, 8:29 AM
brzycki added inline comments to D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.
Mar 7 2018, 8:28 AM
brzycki updated the summary of D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.
Mar 7 2018, 8:11 AM

Mar 6 2018

brzycki added a comment to D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.

Here are 4 ctmark runs on x86_64 of compile_time results. The tip compiler is llvm-project SHA 7a365bd12dd84be1e7c9ca6d9a2268b80f604ae9. The upstream compiler is the same as tip with this patch applied.

Mar 6 2018, 4:15 PM
brzycki created D44177: [JumpThreading] use UnreachableBlocks to avoid unreachable regions.
Mar 6 2018, 3:57 PM
brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.

I have analyzed this failure and have what I believe (in @dberlin 's words) to be the smallest hammer fix:

$ git diff lib/Analysis/LazyValueInfo.cpp
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 65fd007dc0b..36e66f7f6ef 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -1145,9 +1145,17 @@ getValueFromConditionImpl(Value *Val, Value *Cond, bool isTrueDest,
              (!isTrueDest && BO->getOpcode() != BinaryOperator::Or))
     return ValueLatticeElement::getOverdefined();
Mar 6 2018, 2:18 PM

Feb 26 2018

brzycki accepted D41298: [Dominators] Remove verifyDomTree and add some verifying for Post Dom Trees.

+1 LGTM as well.

Feb 26 2018, 8:04 AM

Feb 21 2018

brzycki closed D37575: [BasicBlock] add new function removeEdge().

This method is too expensive as it is designed today.

Feb 21 2018, 8:07 AM

Feb 19 2018

brzycki added a comment to D43450: [GSoC] Dominators project proposal.

+1 from me. I'd love to see a single interface for all Dominator-related analysis or updates. This idea reminds me (a bit) of a comment @dberlin made in D43173 about an interface similar to getBestSimplifyQuery to reduce the number of functions that need explicit pointers to a DT, DDT, PDT or other analysis object. I don't quite understand how this would work but I wanted to bring it up in case the work could be done in a way to anticipate such an interface to reduce re-work.

Feb 19 2018, 1:15 PM

Feb 16 2018

brzycki committed rL325356: [JumpThreading] PR36133 enable/disable DominatorTree for LVI analysis.
[JumpThreading] PR36133 enable/disable DominatorTree for LVI analysis
Feb 16 2018, 8:38 AM
brzycki closed D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).
Feb 16 2018, 8:38 AM

Feb 15 2018

brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

It's looks ok for me but I have not contributed enough to give lgtms.

Feb 15 2018, 11:00 AM

Feb 14 2018

brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

@a.elovikov and @dberlin does this look good to commit?

Feb 14 2018, 11:29 AM
brzycki updated the diff for D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

Added asserts to enableDT / disableDT to verify both DT and DisabledDT are never both valid pointers. Added a final LVI->enabledDT() at the very end of runImpl to allow subsequent passes to use DT with the cached LVI object.

Feb 14 2018, 8:35 AM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

I might be missing something, but why don't we enable DT back in LVI after the JT pass finishes and flushes the DDT? Is it possible that we end up with disabled DT *after* the JT?

Feb 14 2018, 8:31 AM

Feb 13 2018

brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

IMHO, if you were going to go that route, LVI checks whether DT is null, and so you could wrap this all in a "UpdatedDTOrNull" object that, when cast to a DominatorTree *, returned the tree if it was up to date, or a null pointer if it was not.

It's a bit more subtle than that @dberlin due to the way LazyValueInfo is defined. Because the LazyValueInfoImpl object is cached and re-used it would store a pointer to DT (or nullptr) at the instance of creating the first cachced LVI instance. I ran into this problem with my first attempt to disable DT within LVI and is another reason I decided on the new external methods.

Feb 13 2018, 3:22 PM
brzycki updated the diff for D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

Use std::swap() in enableDT/DisableDT methods.

Feb 13 2018, 3:18 PM
brzycki updated the diff for D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

Rebased to tip, switched to always enabling or disabling before calling LVI analysis. Prevents LVI from being accidentally enabled.

Feb 13 2018, 2:12 PM
brzycki added inline comments to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).
Feb 13 2018, 8:21 AM

Feb 12 2018

brzycki added a comment to D43216: [Dominators] Ensure that every PostDomTree root is connected to the virtual node.

Hi @kuhar , is there a testcase that causes this to fail?

Feb 12 2018, 6:32 PM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

I used clang -O3 -fno-exceptions -ftime-report -c tramp3d-v4.cpp -o /tmp/foo.o with an upstream (non-patched) compiler. Here are the hottest areas of pass execution according to the reports. These lines are displayed in the same order:

Feb 12 2018, 4:34 PM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

Six runs of tramp3d-v4 with this patch applied show comperable variance and similar low/high bounds:

Feb 12 2018, 2:09 PM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

The variance of tramp3d-v4 is considerable, as much as 18.5%. I ran CTMark on the pre-DDT compiler and here are the results:

Feb 12 2018, 1:27 PM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

I'm seeing considerable variance on the tramp3d-v4 compile time test before and after the preservation of DT on JumpThreading patch:

Feb 12 2018, 11:56 AM
brzycki added inline comments to D43173: [CallSiteSplitting] Preserve DominatorTreeAnalysis..
Feb 12 2018, 9:39 AM
brzycki added a comment to D43140: [Dominators] Always recalculate postdominators when update yields different roots.

I haven't checked, but from my previous experiments I doubt it does -- forming or to infinite loops are very rare, and this piece of code was only executed a few times when compiling clang or sqlite. I don't have numbers for it, but could get them later this week if someone's really interested.

Feb 12 2018, 7:53 AM
brzycki accepted D43140: [Dominators] Always recalculate postdominators when update yields different roots.

LGTM. Out of curiosity does this change impact compile time?

Feb 12 2018, 7:43 AM

Feb 9 2018

brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

@dberlin thank you for the suggestion and I have implemented the change. The good news is sqlite is back to our original performance. The not-so-great news is this causes a regression for tramp3d-v4:

Feb 9 2018, 9:10 AM
brzycki updated the diff for D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

Added a feature to LVI to enable or disable the use of the DominatorTree for analysis. Updated JumpThreading to disable the use of DT for LVI analysis when there are pending updates in the DDT.

Feb 9 2018, 9:05 AM

Feb 7 2018

brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

@kuhar and @dberlin I would appreciate any guidance you two may have. I spoke with @sebpop about this problem and we don't see a way around the performance impact to preserve DT for LVI analysis. As it stands now there are cases where the deferred updates change the LVI analysis sufficiently to cause a miscompile. We do think there may be ways to mitigate this loss in the future: one way is to prevent pointless threads where a BB is threaded only to be discarded in a subsequent loop iteration within JumpThreading.

Feb 7 2018, 1:33 PM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

Here is a comparison of a run of LLVM before the DDT patch was applied to JT vs upstream.

brzycki@cc01 /work/brzycki/test-suite $ ~/git/me/llvm/litdiff build.pre_dt/results.json build.upstream/results.json compile_time
#  left column: build.pre_dt/results.json
# right column: build.upstream/results.json
#  metric name: compile_time
    527.9440 -> 493.8600     [  6.90%]  CTMark/tramp3d-v4/tramp3d-v4.test
   1203.8440 <- 1265.6800    [  5.14%]  CTMark/7zip/7zip-benchmark.test
    881.3400 <- 924.1160     [  4.85%]  CTMark/Bullet/bullet.test
    576.0160 <- 589.8000     [  2.39%]  CTMark/lencod/lencod.test
    365.8560 <- 373.0960     [  1.98%]  CTMark/consumer-typeset/consumer-typeset.test
    452.0200 <- 459.5960     [  1.68%]  CTMark/SPASS/SPASS.test
    252.3400 <- 256.1360     [  1.50%]  CTMark/mafft/pairlocalalign.test
    491.1680 <- 497.1560     [  1.22%]  CTMark/ClamAV/clamscan.test
    382.8120 <- 387.2960     [  1.17%]  CTMark/kimwitu++/kc.test
    336.8720 -> 336.3600     [  0.15%]  CTMark/sqlite3/sqlite3.test
Feb 7 2018, 9:14 AM

Feb 6 2018

brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

I'm seeing about 3% run variation on sqlite with the upstream compiler:

brzycki@cc01 /work/brzycki/test-suite $ ~/git/me/llvm/litdiff build.upstream/results.json build.upstream2/results.json compile_time
#  left column: build.upstream/results.json
# right column: build.upstream2/results.json
#  metric name: compile_time
    336.3600 <- 347.4120     [  3.29%]  CTMark/sqlite3/sqlite3.test
    493.8600 -> 487.0560     [  1.40%]  CTMark/tramp3d-v4/tramp3d-v4.test
    373.0960 -> 368.5280     [  1.24%]  CTMark/consumer-typeset/consumer-typeset.test
    387.2960 <- 389.8320     [  0.65%]  CTMark/kimwitu++/kc.test
    459.5960 <- 462.5440     [  0.64%]  CTMark/SPASS/SPASS.test
    924.1160 -> 921.2680     [  0.31%]  CTMark/Bullet/bullet.test
    497.1560 <- 498.5240     [  0.28%]  CTMark/ClamAV/clamscan.test
    589.8000 -> 588.3840     [  0.24%]  CTMark/lencod/lencod.test
   1265.6800 <- 1267.2280    [  0.12%]  CTMark/7zip/7zip-benchmark.test
    256.1360 -> 256.1000     [  0.01%]  CTMark/mafft/pairlocalalign.test
Feb 6 2018, 3:49 PM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

@kuhar I had a bug in my test-suite runner script and just fixed it. The new results are require more analysis, particularly sqlite.

Feb 6 2018, 3:35 PM

Feb 2 2018

brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

That seems to be within noise and not an issue

Feb 2 2018, 12:57 PM
brzycki added a comment to D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).

Thank you @kuhar for the review. I ran CTmark (x86_64) on tip today and with this patch applied. The results are:

Feb 2 2018, 12:33 PM

Jan 31 2018

brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.

I have a patch (that relies on my fix from D42717) to also fix this issue with little additional overhead. Because we must flush all pending DT transactions before using LVI analysis we can also check if the block is unreachable from entry. If that's the case we can immediately pessimize threads across the block. Here's the patch along with a modified version of D42717:

Jan 31 2018, 8:37 AM

Jan 30 2018

brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.

(also, the DT was not updated at all prior during JT prior to all of these patches, so i guess it all worked because LVI didn't use DT when called from JumpThreading or something)?

Jan 30 2018, 4:06 PM
brzycki created D42717: [JumpThreading] sync DT for LVI analysis (PR 36133).
Jan 30 2018, 4:00 PM

Jan 29 2018

brzycki committed rL323695: [JumpThreading][NFC] Rename LoadInst variables.
[JumpThreading][NFC] Rename LoadInst variables
Jan 29 2018, 1:31 PM
brzycki closed D42601: [JumpThreading] NFC: Rename LoadInst variables.
Jan 29 2018, 1:31 PM

Jan 26 2018

brzycki created D42601: [JumpThreading] NFC: Rename LoadInst variables.
Jan 26 2018, 2:07 PM

Jan 23 2018

brzycki accepted D42406: Utils: Fix DomTree update for entry block.

I too am curious how this managed to not be found for several years. @arsenm how did you discover this?

I'm working on a replacement for StructurizeCFG which needs to insert a block before certain blocks. Sometimes that ends up picking the entry block

Jan 23 2018, 10:54 AM
brzycki added a comment to D42406: Utils: Fix DomTree update for entry block.

I too am curious how this managed to not be found for several years. @arsenm how did you discover this?

Jan 23 2018, 8:17 AM

Jan 22 2018

brzycki accepted D42337: [Dominators] Introduce DomTree verification levels .

LGTM.

Jan 22 2018, 11:34 AM
brzycki added a comment to D42337: [Dominators] Introduce DomTree verification levels .

LGTM. I just have one question about the patch, see inline.

Jan 22 2018, 7:54 AM

Jan 19 2018

brzycki accepted D42231: [Dominators] Visit affected node candidates found at different root levels .

LGTM. I've thrown everything I can think of to attempt to trigger a DT regression and found no asserts().

Jan 19 2018, 10:49 AM
brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

@MatzeB wrote:
Some files like sqlite3.c felt like they would never finish...

Jan 19 2018, 8:34 AM
brzycki abandoned D38558: [JumpThreading] Preserve DT and LVI across the pass..

This code is superceded by D40146.

Jan 19 2018, 8:24 AM

Jan 18 2018

brzycki added a comment to D42231: [Dominators] Visit affected node candidates found at different root levels .

The changes to GenericDomTreeConstruction.h look good to me and I verified the new unit test fails without the patch. Other than my issue with ddt-crash3.ll the patch looks good.

Jan 18 2018, 11:19 AM
brzycki added a comment to D42231: [Dominators] Visit affected node candidates found at different root levels .

Your patch to GenericDomTreeConstruction.h passes ninja check-all on x86_64 as well as test-suite on x86_64. I used my version of ddt-crash3.ll posted above for the testing.

Jan 18 2018, 9:43 AM
brzycki updated subscribers of D42231: [Dominators] Visit affected node candidates found at different root levels .

Hi @kuba, I was unable to recreate the crash with the version of ddt-crash3.ll included in this patch. Here's my version of ddt-crash3.ll that fails before the patch. I've also verified this version passes after your patch. I'll take my time and review the real patch later today.

Jan 18 2018, 9:26 AM

Jan 17 2018

brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.

Added inline comment.

Jan 17 2018, 11:48 AM
brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.

Or should we add

if (!DDT->flush().isReachableFromEntry(BB))
  continue;

?
That alone seems to solve the problem, even if I remove the fix in ProcessBlock.

Jan 17 2018, 11:46 AM

Jan 16 2018

brzycki added a comment to D34135: [JumpThreading] Use DT to avoid processing dead blocks.
Jan 16 2018, 8:05 AM

Jan 12 2018

brzycki committed rL322401: [JumpThreading] Preservation of DT and LVI across the pass.
[JumpThreading] Preservation of DT and LVI across the pass
Jan 12 2018, 1:08 PM
brzycki closed D40146: [JumpThreading] Preservation of DT and LVI across the pass.
Jan 12 2018, 1:08 PM
brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Preparing to commit to SVN.

Jan 12 2018, 12:46 PM
brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Fixed punctuation in unit test comment. NFC.

Jan 12 2018, 12:46 PM
brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Update nits in the unit test for clarity.

Jan 12 2018, 12:16 PM
brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

@kuba Thank you again for the careful review. I'm uploading a new diff shortly. If you could give it a quick LGTM I'll try to push the patch to tip again today.

Jan 12 2018, 12:15 PM

Jan 11 2018

brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Fixed comments in unit test.

Jan 11 2018, 2:26 PM

Jan 10 2018

brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Fixed Chrome build patch with suggestions from @kuhar . Added a first pass at DDT unit tests.

Jan 10 2018, 1:27 PM

Jan 9 2018

brzycki updated subscribers of D40146: [JumpThreading] Preservation of DT and LVI across the pass.

I'm OK with reapplying this, but when it causes a couple more crashes I'd strongly consider some more exhaustive testing approach.

Thanks @kuhar for the review. I agree with you that DDT should have a unittest module and I've started work on it. I have the skeleton in place and am working on the actual tests now. I'm still trying to determine the set of unique CFG shapes/mutations to test for.

Jan 9 2018, 2:16 PM

Jan 8 2018

brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

@kuhar if you have the time I would appreciate your feedback on the fix and if you're still ok with the patch.

Jan 8 2018, 2:14 PM
brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Fxed assert() caused when compiling Chrome.

Jan 8 2018, 11:46 AM

Jan 5 2018

brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

The patch was reverted in r321832 by @rnk for the following reason:

This reverts r321825, it causes crashes in Chromium. Reproducer forthcoming.

Jan 5 2018, 9:29 AM
brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.
In D40146#967965, @rnk wrote:

CReduce got this reduction, but I think you can go further:

Thank you @rnk for reducing the test case. I don't think you need to go further, I'll convert to llvm bytecode inside gdb and reduce with bugpoint once I verify the crash locally. Once there I'll post the .ll.

Jan 5 2018, 8:49 AM
brzycki reopened D40146: [JumpThreading] Preservation of DT and LVI across the pass.

The patch was reverted in r321832 by @rnk for the following reason:

Jan 5 2018, 8:47 AM
brzycki closed D37528: [JumpThreading] Preserve DT and LVI across the pass..

I accidentally re-opened the wrong diff.

Jan 5 2018, 8:34 AM
brzycki reopened D37528: [JumpThreading] Preserve DT and LVI across the pass..
Jan 5 2018, 8:32 AM

Jan 4 2018

brzycki committed rL321825: [JumpThreading] Preservation of DT and LVI across the pass.
[JumpThreading] Preservation of DT and LVI across the pass
Jan 4 2018, 1:58 PM
brzycki closed D40146: [JumpThreading] Preservation of DT and LVI across the pass.
Jan 4 2018, 1:58 PM

Jan 3 2018

brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Getting patch ready to commit again. Rebased to llvm tip, rancheck, check-all, and test-suite on x86_64. Also passed building with LLVM_ENABLE_MODULES=1.

Jan 3 2018, 9:07 AM

Jan 2 2018

brzycki added a comment to D41609: NFC. Add description comments to Function header.

It looks ok to me. I’d prefer the comments reflect the overloaded method parameters but it’s not necessary.

I've looked through other places with similar semantics, and it's quite common that comments do not reflect parameters type, so I decided to make it that way.

Jan 2 2018, 8:54 AM

Dec 28 2017

brzycki accepted D41609: NFC. Add description comments to Function header.

It looks ok to me. I’d prefer the comments reflect the overloaded method parameters but it’s not necessary.

Dec 28 2017, 3:51 PM

Dec 18 2017

brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Update patch with a minimal class definition of DeferredDominance in Dominators.h. Also removed an unnecessary #include of Dominators.h in JumpThreading.h

Dec 18 2017, 9:27 AM
brzycki updated subscribers of D40146: [JumpThreading] Preservation of DT and LVI across the pass.

I wonder what caused the original cycle. I'm not entirely sure how modules are built, but have you tried getting rid of the include in Local.h? If that's not enough to break the cycle, I don't have any idea better than yours.

It's a bit of a mystery to me how -fmodules actually works. I am getting cyclic errors when compiling a file that does not include either of Dominators.h or DeferredDominance.h. I have emailed @rsmith requesting his expertise if he can help us understand this.

Dec 18 2017, 8:42 AM

Dec 15 2017

brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Which files and includes formed the cycle? There are a lot of files that require dominators.h, and I'm not convinced adding a new class to is a good idea, given that it currently has just one user.

Dec 15 2017, 7:38 AM

Dec 14 2017

brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Here is another failure linking it directly to DeferredDominance.h:

Dec 14 2017, 3:42 PM
brzycki added a comment to D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Which files and includes formed the cycle? There are a lot of files that require dominators.h, and I'm not convinced adding a new class to is a good idea, given that it currently has just one user.

Dec 14 2017, 3:39 PM
brzycki updated the diff for D40146: [JumpThreading] Preservation of DT and LVI across the pass.

Refactored patch to prevent -fmodules circular dependency errors.

Dec 14 2017, 2:28 PM
brzycki reopened D40146: [JumpThreading] Preservation of DT and LVI across the pass.

This patch caused a build break when attempting to use -fmodules due to a cyclic header dependency chain. I've refactored the patch to no longer use DeferredDominance.h (the code now lives as a class in Dominators.h). I also moved one of the methods into Dominators.cpp that needed Constants.h and Instructions.h. @sebpop and @kuhar could you please review again?

Dec 14 2017, 2:27 PM

Dec 13 2017

brzycki committed rL320641: Reverting [JumpThreading] Preservation of DT and LVI across the pass.
Reverting [JumpThreading] Preservation of DT and LVI across the pass
Dec 13 2017, 2:02 PM
brzycki committed rL320612: [JumpThreading] Preservation of DT and LVI across the pass.
[JumpThreading] Preservation of DT and LVI across the pass
Dec 13 2017, 12:53 PM
brzycki closed D40146: [JumpThreading] Preservation of DT and LVI across the pass.
Dec 13 2017, 12:53 PM