Page MenuHomePhabricator

reames (Philip Reames)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 23 2013, 6:32 PM (399 w, 5 d)

Recent Activity

Today

reames requested review of D104665: [instcombine] Fold overflow check using umulo to comparison.
Mon, Jun 21, 12:51 PM · Restricted Project
reames added a comment to D104662: [SCEVExpander] Prefer pointer expansion for overflow checks.

Do we need to check that GEP index size is big enough for this?

I don't believe so, but maybe I'm missing something?

Mon, Jun 21, 12:36 PM · Restricted Project
reames updated the diff for D104662: [SCEVExpander] Prefer pointer expansion for overflow checks.

Corrected patch.

Mon, Jun 21, 12:06 PM · Restricted Project
reames requested review of D104662: [SCEVExpander] Prefer pointer expansion for overflow checks.
Mon, Jun 21, 12:05 PM · Restricted Project
reames committed rG0c09e5bd74db: Split a test for ease of auto update (authored by reames).
Split a test for ease of auto update
Mon, Jun 21, 11:03 AM
reames accepted D103238: [RS4GC] Add a test to demonstrate duplication of base generation. NFC.

p.s. This could have landed without review.

Mon, Jun 21, 10:37 AM · Restricted Project
reames added a comment to D103959: [LoopDeletion] Handle Phis with similar inputs from different block.

@nathanchance - I'd suggest reverting until @mkazantsev can investigate. He's offline for the next few hours due to time zones.

Mon, Jun 21, 10:00 AM · Restricted Project
reames accepted D104590: [LoopUnroll] Don't modify TripCount/TripMultiple in computeUnrollCount().

Reading through the code again, I think I agree with your analysis.

Mon, Jun 21, 9:38 AM · Restricted Project
reames added inline comments to D104547: [langref] attempt to clarify semantics of inttoptr/ptrtoint for non-integral types.
Mon, Jun 21, 9:35 AM · Restricted Project
reames added a comment to D104503: [SCEV] Don't require dominance ordering of add/mul/min/max expressions.

There are two relevant questions:

  1. Does SCEV do something reasonable with such an expression? I mean, you obviously can't expand it; there isn't any viable insertion point. More generally, I'm not sure it's meaningful; even if we drop the assertions, it's not clear what the result is supposed to represent. If we want to support "subtracting" two addrecs to do dependency analysis, we should probably add a dedicated method, which returns something that isn't a regular SCEV expression.
  2. Why is SCEVAA trying to construct such an expression? AliasAnalysis::alias() doesn't make any sense if there isn't a dominance relationship between the two pointers.

On (1), I think that point has convinced me we need to not create such scevs and return could not compute instead. Will give it a bit more thought, but that's the way I'm currently leaning.

Mon, Jun 21, 9:32 AM · Restricted Project
reames accepted D103656: [ScalarEvolution] Ensure backedge-taken counts are not pointers..

For the record, some of the test diffs are a bit concerning code quality wise, but I think we may need to accept them to achieve the goal. At least a couple of them have obvious possibilities for improvement in follow ups if needed, so I'm willing to have this go in. We may end up having to revert, fix a few things, then reapply though. We'll see.

Mon, Jun 21, 9:29 AM · Restricted Project
reames added a comment to D104618: [LoopDeletion] Exploit undef when symbolically executing 1st iteration.

I would suggest splitting this into two patches. The "ignore undef input in phi" is safe, and commonly done across the optimizer. The "branch on undef" is tricky. Just from a risk management perspective, I think it makes sense to split them into separate changes.

Mon, Jun 21, 9:20 AM · Restricted Project
reames added a comment to D104634: [SCEV] Generalize MatchBinaryAddToConst to support non-add expressions..

For an optional follow up change, I believe we can avoid checking the wrap flags for one operand, if C1 and C2 share the same sign.

Mon, Jun 21, 9:15 AM · Restricted Project

Yesterday

reames added a comment to D102982: [LoopUnroll] Use smallest exact trip count from any exit.

Comment much improved, thanks!

Sun, Jun 20, 11:24 AM · Restricted Project

Sat, Jun 19

reames accepted D102982: [LoopUnroll] Use smallest exact trip count from any exit.

Response explained the test case interaction. LGTM w/a tweaked comment.

Sat, Jun 19, 5:08 PM · Restricted Project
reames added a comment to D102982: [LoopUnroll] Use smallest exact trip count from any exit.

Would be LGTM, but I'd like to understand the called out test change first. Everything else looks as expected.

Sat, Jun 19, 10:15 AM · Restricted Project
reames added inline comments to D104590: [LoopUnroll] Don't modify TripCount/TripMultiple in computeUnrollCount().
Sat, Jun 19, 10:02 AM · Restricted Project

Fri, Jun 18

reames accepted D104482: [LoopUnroll] Simplify optimization remarks.

LGTM

Fri, Jun 18, 1:28 PM · Restricted Project
reames added a comment to D104482: [LoopUnroll] Simplify optimization remarks.

I think I would prefer to see the information for each exit as opposed to removing it entirely. When understanding why unrolling happened in a particular way, having the detailed information is often very helpful. Not a strong preference, I'm willing to LGTM this if you really object.

I'm not particularly familiar with how optimization remarks work -- if we wanted to go the other way around, we need to be able to identify specific exits in some way. For debug output, we can just print the block label, but I assume that wouldn't be suitable for optimization remarks, as the block label wouldn't really be meaningful in that context, it's an IR detail. Do you know how one would go about that?

Hm, good point I was thinking in terms of debug output, and hadn't recognized the ORE complication.

Fri, Jun 18, 11:32 AM · Restricted Project
reames requested changes to D103656: [ScalarEvolution] Ensure backedge-taken counts are not pointers..

This looks really close to good to go.

Fri, Jun 18, 10:00 AM · Restricted Project
reames added a comment to D104403: [SCEV] Avoid pointer subtraction of non-integral pointers [WIP].

FYI, agree with all the stylistic comments from Eli. Waiting on perf numbers to see if this is worth cleaning up, or if we need a radically different approach.

Fri, Jun 18, 9:51 AM · Restricted Project
reames planned changes to D104503: [SCEV] Don't require dominance ordering of add/mul/min/max expressions.

Max is correct. I think I can handle that easily, but let me make sure and then repost.

Fri, Jun 18, 9:49 AM · Restricted Project
reames requested changes to D104482: [LoopUnroll] Simplify optimization remarks.

I think I would prefer to see the information for each exit as opposed to removing it entirely. When understanding why unrolling happened in a particular way, having the detailed information is often very helpful. Not a strong preference, I'm willing to LGTM this if you really object.

Fri, Jun 18, 9:49 AM · Restricted Project
reames accepted D104487: [LoopUnroll] Push runtime unrolling decision up into tryToUnrollLoop().

LGTM. Nice cleanup.

Fri, Jun 18, 9:43 AM · Restricted Project
reames added a reviewer for D104547: [langref] attempt to clarify semantics of inttoptr/ptrtoint for non-integral types: samparker.
Fri, Jun 18, 9:34 AM · Restricted Project
reames requested review of D104547: [langref] attempt to clarify semantics of inttoptr/ptrtoint for non-integral types.
Fri, Jun 18, 9:34 AM · Restricted Project

Thu, Jun 17

reames requested review of D104503: [SCEV] Don't require dominance ordering of add/mul/min/max expressions.
Thu, Jun 17, 6:46 PM · Restricted Project

Wed, Jun 16

reames accepted D104203: [LoopUnroll] Fold all exits based on known trip count/multiple.

LGTM, concerns addressed by responses. The bit about which exits we fold is particularly subtle. Might be worth adding a comment to the code.

Wed, Jun 16, 7:48 PM · Restricted Project
reames accepted D103660: [ScalarEvolution] Fix pointer/int type handling converting select/phi to min/max..

LGTM

Wed, Jun 16, 2:03 PM · Restricted Project
reames added a comment to D104322: [SCEV] PtrToInt on non-integral pointers is allowed.

Took a shot at a minimal crippling of SCEV over in https://reviews.llvm.org/D104403. That isn't a patch ready to land, but it should let us collect some perf numbers on the impact of crippling SCEV's ability to reason about non-integral pointers.

Wed, Jun 16, 11:03 AM · Restricted Project
reames requested review of D104403: [SCEV] Avoid pointer subtraction of non-integral pointers [WIP].
Wed, Jun 16, 11:00 AM · Restricted Project
reames added inline comments to D103660: [ScalarEvolution] Fix pointer/int type handling converting select/phi to min/max..
Wed, Jun 16, 9:11 AM · Restricted Project
reames added a comment to D104322: [SCEV] PtrToInt on non-integral pointers is allowed.

As a drive-by note, it would be great if you could expand LangRef on non-integral pointers a bit. It made sense to me when it specified that you can't use ptrtoint on a non-integral pointer, but without that limitation, it's not really clear to me what the actual difference between a non-integral pointer and a normal one is. What transforms are you not allowed to perform on a non-integral pointer that you can perform on a normal one?

Yes, i would also like to see such documentation, especially if non-integral pointers
are going to be used as an "arbitrary" roadblock for SCEV changes. I would have posted
that in the review for the commit mentioned, but there was none, which also highlights
the problem around non-integral pointer status in llvm :)

On the documentation side, I'd love to, but I'm honestly not sure *how* to. There's two inter-related problems here. The first is the semantics of an inttoptr is highly dependent on the target for non-integral pointers. At the moment, it can basically only be used to implement non-inlined built-in routines in any practical way. The second issue is that our definition of the integral pointer types themselves appear to be in flux, and are very vague about certain key details. The result is I'm left unsure how to formally specify them. This is why I used the implementation defined wording I did.

Wed, Jun 16, 8:57 AM · Restricted Project
reames added a comment to D103660: [ScalarEvolution] Fix pointer/int type handling converting select/phi to min/max..

The revised change looks a lot more reasonable. With some cleanup, I'd be willing to LGTM this. I'm much happier with the explicit focus on avoiding the construction of a subtract of pointer type.

Wed, Jun 16, 8:36 AM · Restricted Project
reames added a comment to D104322: [SCEV] PtrToInt on non-integral pointers is allowed.

JFYI, I don't object to this change, but I am hesitant about the direction you seem to be indicating. Please don't use the excuse of our handling of non-integral types being somewhat broken to further break them. I'll comment on future reviews if appropriate.

Wed, Jun 16, 8:17 AM · Restricted Project
reames added a comment to D103656: [ScalarEvolution] Ensure backedge-taken counts are not pointers..

Having those queries return unknown more often would be undesirable.

I agree.

Second, the getLosslessPtrToIntExpr routine seems to not match what you're describing. It appears to have a bunch of cases where it does not succeed. Examples include min/max expressions, and udivs. (Going by code structure here. If I'm missing something, let me know.) Are you claiming that this routine is guaranteed to succeed? The existing code and comments don't seem to make this obvious.

SCEVPtrToIntSinkingRewriter always succeeds; it inherits implementations for every kind of expression from SCEVRewriteVisitor. The explicit implementations of visitAddExpr and visitMulExpr are just there to preserve the flags. Not sure how you're reaching the conclusion that it fails for min/max expressions.

Reading back through, I see you are correct. The only bailout (e.g. return of SCEVCouldNotCompute) is the effective type size check. Can you update/add some comments to the function to that effect?

Wed, Jun 16, 8:14 AM · Restricted Project

Tue, Jun 15

reames added a comment to D103660: [ScalarEvolution] Fix pointer/int type handling converting select/phi to min/max..

The goal stated of having getSCEV(V)->getType()->isPointerTy() == V->getType()->isPointerTy() seems reasonable to me. I'm not as sure about the baseof(V) == baseof(S) bit, but I tentatively accept that as it's not my main confusion.

Tue, Jun 15, 7:40 PM · Restricted Project
reames added a comment to D103656: [ScalarEvolution] Ensure backedge-taken counts are not pointers..

Sorry for the silence. Partly busy, and partly having a hard time explaining something which seems obvious to me. Let me give this another try.

Tue, Jun 15, 7:22 PM · Restricted Project
reames added a comment to D102615: [LoopDeletion] Break backedge if we can prove that the loop is exited on 1st iteration (try 3).

I want to chime in on the philosophy bits raised in the last few comments.

Tue, Jun 15, 6:43 PM · Restricted Project
reames added a comment to D104319: [SCEV] Retain AddExpr flags when subtracting a foldable constant..

I agree w/Eli's comments.

Tue, Jun 15, 3:50 PM · Restricted Project
reames resigned from D104148: [LoopUtils] Fix incorrect RT check bounds for loop-invariant mem accesses.

Not familiar with this code.

Tue, Jun 15, 3:46 PM · Restricted Project
reames added a comment to D104203: [LoopUnroll] Fold all exits based on known trip count/multiple.

I think there's a problem with this code, but I'm surprised the existing test didn't catch it. Have you run this with explicit dom tree and loop info validation enabled?

Tue, Jun 15, 3:39 PM · Restricted Project
reames added inline comments to D104140: [SCEV] Allow negative steps for LT exit count computation.
Tue, Jun 15, 12:15 PM · Restricted Project
reames added inline comments to D104140: [SCEV] Allow negative steps for LT exit count computation.
Tue, Jun 15, 8:58 AM · Restricted Project

Mon, Jun 14

reames added inline comments to D104140: [SCEV] Allow negative steps for LT exit count computation.
Mon, Jun 14, 9:54 AM · Restricted Project

Fri, Jun 11

reames added a comment to D103492: [RS4GC] Treat inttoptr as base pointer.

For those following along, the last comment from Sam convinced me to do something I've been considering for a while. In commit ac81cb7e, I removed the verifier rules which made it impossible to properly test the logic in rs4gc with non-integral pointers and went ahead and updated the relevant tests.

Fri, Jun 11, 1:44 PM · Restricted Project
reames committed rGac81cb7e6dde: Allow ptrtoint/inttoptr of non-integral pointer types in IR (authored by reames).
Allow ptrtoint/inttoptr of non-integral pointer types in IR
Fri, Jun 11, 1:40 PM
reames accepted D100559: [GC][NFC] Make getGCStrategy by name available in IR.

Max, I'd still prefer the singleton, but frankly, I don't care enough to keep discussing this. Land what we've got; we can refactor at some point in the future.

Fri, Jun 11, 12:53 PM · Restricted Project
reames added a comment to D103887: [StackMaps] Force section serialization for functions with no records.

I don't really have an objection to this, but have you looked at the -stack-size-section support? I think you might be able to get the same information that way. If that doesn't work for some reason, just let me know and we can move forward with the review of this.

Fri, Jun 11, 12:51 PM · Restricted Project
reames added inline comments to D103844: [SCEV] Cache wrap facts for positive IVs w/LT exits.
Fri, Jun 11, 12:47 PM · Restricted Project
reames added inline comments to D103844: [SCEV] Cache wrap facts for positive IVs w/LT exits.
Fri, Jun 11, 12:45 PM · Restricted Project
reames requested review of D104140: [SCEV] Allow negative steps for LT exit count computation.
Fri, Jun 11, 11:35 AM · Restricted Project
reames added a comment to D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit.

@Ayal If I'm understanding you even partially correctly, it sounds like you're raising a code quality issue. That is, we may generate a dead epilogue loop (e.g. with a condition statically known to result in the epilogue being untaken, but emitted as a condition) when we didn't need to. Is that correct?

Fri, Jun 11, 11:25 AM · Restricted Project
reames added a comment to D97982: [MC] Introduce NeverAlign fragment type.

Drive by thought, not intended to be blocking.

Fri, Jun 11, 10:41 AM · Restricted Project
reames updated subscribers of D97982: [MC] Introduce NeverAlign fragment type.

@lebedev.ri @reames Could you weigh in from performance perspective?

JFYI, I no longer have access to the infrastructure used to test previous patches in this area. You could consider asking @skatkov if needed.

Fri, Jun 11, 10:33 AM · Restricted Project
reames planned changes to D103844: [SCEV] Cache wrap facts for positive IVs w/LT exits.

Change appears to be wrong as written, discussing why before adjusting.

Fri, Jun 11, 10:30 AM · Restricted Project
reames added inline comments to D103991: [SCEV] Extend mustprogress reasoning to ne exit tests.
Fri, Jun 11, 10:12 AM · Restricted Project
reames requested changes to D104075: [ScalarEvolution] Merge howManyGreaterThans with howManyLessThans..

Several level of comments.

Fri, Jun 11, 9:55 AM · Restricted Project
reames added a comment to D104066: [SCEV] Use knowledge of stride to prove loops finite for LT exit count computation.

I agree with the request to hold until latent miscompile in underlying code is addressed. I will hold this patch until that happens, and then land without further review. Max, good suggestion on condition ordering, will address in landed patch.

Fri, Jun 11, 9:49 AM · Restricted Project

Thu, Jun 10

reames requested review of D104066: [SCEV] Use knowledge of stride to prove loops finite for LT exit count computation.
Thu, Jun 10, 2:57 PM · Restricted Project
reames committed rG7629b2a09c16: [LI] Add a cover function for checking if a loop is mustprogress [nfc] (authored by reames).
[LI] Add a cover function for checking if a loop is mustprogress [nfc]
Thu, Jun 10, 1:37 PM
reames added a comment to D103834: [SCEV] Properly guard reasoning about infinite loops being UB on mustprogress.

The regression for C programs should be addressed in aaaeb4b16. Note that this requires code motion done in b6ee5f2b.

Thu, Jun 10, 1:21 PM · Restricted Project
reames committed rGaaaeb4b160fe: [SCEV] Use mustprogress flag on loops (in addition to function attribute) (authored by reames).
[SCEV] Use mustprogress flag on loops (in addition to function attribute)
Thu, Jun 10, 1:20 PM
reames committed rGb6ee5f2b1df6: Move code for checking loop metadata into Analysis [nfc] (authored by reames).
Move code for checking loop metadata into Analysis [nfc]
Thu, Jun 10, 1:01 PM
reames added a comment to D103834: [SCEV] Properly guard reasoning about infinite loops being UB on mustprogress.

JFYI, C and C++ apparently have different rules here. For c++ your example function is mustprogress, for C only the loop is. (Based on a quick skim of checkIfLoopMustProgress clang's CodeGenFunction.h) I have no idea if the C rules could be more aggressive here or not, you'd be best taking that up with a clang developer.

Thu, Jun 10, 12:04 PM · Restricted Project

Wed, Jun 9

reames updated subscribers of D101722: [SCEV] Don't require ControlsExit for gt/lt NoWrap.

Here is what I think an example of a bug in the current computeBECount. Overflow is complicated, so I might be wrong here. Sanity check me?

Wed, Jun 9, 8:58 PM · Restricted Project
reames committed rG4ac3dae57f27: [tests] Precommit test for D103991 (authored by reames).
[tests] Precommit test for D103991
Wed, Jun 9, 3:06 PM
reames requested review of D103991: [SCEV] Extend mustprogress reasoning to ne exit tests.
Wed, Jun 9, 2:48 PM · Restricted Project
reames committed rGb65f30d6fb6f: [SCEV] Minor code motion to simplify a later patch [nfc] (authored by reames).
[SCEV] Minor code motion to simplify a later patch [nfc]
Wed, Jun 9, 2:17 PM
reames added a comment to D101722: [SCEV] Don't require ControlsExit for gt/lt NoWrap.

@nikic I'm increasingly thinking that the computeBECount logic is just wrong. I believe you can trip the overflow to zero case with a loop with explicit (correct) flags. There's some comments and a guard on negative steps which appear incorrect, and I'm guessing were an attempt to work around an existing bug.

Wed, Jun 9, 2:12 PM · Restricted Project
reames added inline comments to D103844: [SCEV] Cache wrap facts for positive IVs w/LT exits.
Wed, Jun 9, 2:08 PM · Restricted Project

Mon, Jun 7

reames committed rG3c6e419198f3: [SCEV] Properly guard reasoning about infinite loops being UB on mustprogress (authored by reames).
[SCEV] Properly guard reasoning about infinite loops being UB on mustprogress
Mon, Jun 7, 2:48 PM
reames closed D103834: [SCEV] Properly guard reasoning about infinite loops being UB on mustprogress.
Mon, Jun 7, 2:48 PM · Restricted Project
reames requested review of D103844: [SCEV] Cache wrap facts for positive IVs w/LT exits.
Mon, Jun 7, 2:42 PM · Restricted Project
reames requested review of D103834: [SCEV] Properly guard reasoning about infinite loops being UB on mustprogress.
Mon, Jun 7, 11:41 AM · Restricted Project
reames committed rG38540d71c74c: [SCEV] Compute exit counts for unsigned IVs using mustprogress semantics (authored by reames).
[SCEV] Compute exit counts for unsigned IVs using mustprogress semantics
Mon, Jun 7, 11:24 AM
reames closed D103118: [SCEV] Compute exit counts for unsigned IVs using mustprogress semantics.
Mon, Jun 7, 11:24 AM · Restricted Project
reames accepted D103748: [LoopUnroll] Clamp unroll count to MaxTripCount.

LGTM w/one required change.

Mon, Jun 7, 10:40 AM · Restricted Project
reames added a comment to D103492: [RS4GC] Treat inttoptr as base pointer.

When reviewing your https://reviews.llvm.org/D103732 (in particular, the constants.ll test which had inttoptr constant expressions), I realized that this patch is necessary due to unreachable code. I went ahead and landed a slightly tweaked version of this change. I believe you should now be able to rebase your D103732 without needing to XFAIL a test.

Mon, Jun 7, 10:30 AM · Restricted Project
reames committed rGc880d5e583a3: [RS4GC] Treat inttoptr as base pointer (authored by reames).
[RS4GC] Treat inttoptr as base pointer
Mon, Jun 7, 10:27 AM
reames closed D103492: [RS4GC] Treat inttoptr as base pointer.
Mon, Jun 7, 10:27 AM · Restricted Project
reames added a comment to D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit.

We will be happy to test this on both our LE and BE systems. @stefanp do you have time to test this out or should I do it?

Seems like overkill for this one given the change is trivial and the test is target independent.

I'd save that for when I go to reapply the original patch that exposed this.

Oh, sorry. I meant to say that we'd test with this patch and the reverted one applied together.

I added both patches and I did a quick test on the Big Endian Power PC buildbot machine.
This patch does seem to fix the original issue. However, after adding the two patches I have 4 new LIT test failues:

LLVM :: Transforms/LoopVectorize/first-order-recurrence-complex.ll
LLVM :: Transforms/LoopVectorize/loop-form.ll
LLVM :: Transforms/LoopVectorize/multiple-exits-versioning.ll
LLVM :: Transforms/LoopVectorize/unroll_nonlatch.ll

The original patch would definitely need rebased over changed tests, so that's not surprising.

Mon, Jun 7, 9:33 AM · Restricted Project
reames added inline comments to D94892: [LV] Unconditionally branch from middle to scalar preheader if the scalar loop must execute.
Mon, Jun 7, 9:31 AM · Restricted Project
reames added inline comments to D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit.
Mon, Jun 7, 9:28 AM · Restricted Project

Fri, Jun 4

reames requested changes to D103316: Hoist llvm.assume into single predecessor if block otherwise empty.
Fri, Jun 4, 3:21 PM · Restricted Project
reames added inline comments to D103316: Hoist llvm.assume into single predecessor if block otherwise empty.
Fri, Jun 4, 3:21 PM · Restricted Project
reames added a comment to D103316: Hoist llvm.assume into single predecessor if block otherwise empty.

Wrt assume operand bundles I am thinking that those can always simply be dropped.

I think you can simply ignore operand bundle forms. This is not on by default, and the implementation isn't yet mature. The folks working on that can pay the cost of figuring out what to do for them. As long as your code doesn't crash on operand bundle assumes, I don't really care what it does with them.

Fri, Jun 4, 3:15 PM · Restricted Project
reames added a comment to D103316: Hoist llvm.assume into single predecessor if block otherwise empty.

This kind of transformation should probably eventually reside in lib/Transforms/Utils/SimplifyCFG.cpp but for now it seemed easier and a smaller change to just leave it in CodeGenPrepare.

I don't think this transform makes a lot of sense in CGP, I'd prefer it to go directly to SimplifyCFG (after which we can drop the CGP code).

I agree.

I think we would be better off to just drop these assumes. The framing I'm thinking of here is that SimplifyCFG should generally be ignoring ephemeral values in transforms. So if normally SimplifyCFG would drop a block that contains no instructions, then it should also drop a block that only contains ephemeral values.

I disagree.

if (x)
  assume(y)

is a natural way to spell something for the user.
We should not just drop it without having a good reason.
While we might not use assume(x && y) directly, we will use it if we specialize x to true in some places.

Er, I think you misread the comment you were replying to. The comment was discussing assume(A || B), your response is discussing assume(A && B). Given the context of the discussion, that difference seems important.

Fri, Jun 4, 3:14 PM · Restricted Project
reames requested changes to D103660: [ScalarEvolution] Fix pointer/int type handling converting select/phi to min/max..

Can you take a step back and describe the problem you're trying to solve? I've read through the bug, but nothing there immediately makes me think "bug in core part of SCEV" as opposed to "bug in user of SCEV". How'd you get from there to here?

Fri, Jun 4, 2:54 PM · Restricted Project
reames requested changes to D103656: [ScalarEvolution] Ensure backedge-taken counts are not pointers..

The implementation and description of this patch seem badly out of sync.

Fri, Jun 4, 2:44 PM · Restricted Project
reames added a comment to D103492: [RS4GC] Treat inttoptr as base pointer.

I think RS4GC should make sure that the address space it uses has been specified non-integral and give an error if it hasn't, as this seems like an easy trap to fall into.

Good idea, post a patch and it'd be a quick LGTM.

Fri, Jun 4, 2:35 PM · Restricted Project
reames added a comment to D103362: [LoopUnroll] Separate peeling from unrolling.

LGTM still holds.

Fri, Jun 4, 2:23 PM · Restricted Project
reames added a comment to D103424: [IndVars] Don't forget value when inferring nowrap flags.

I went ahead and wrote up my thoughts on the optimizeable IR notion I mentioned upthread. You can read them here: https://github.com/preames/public-notes/blob/master/llvm-loop-opt-ideas.rst#should-scev-be-an-optimizeable-ir

Fri, Jun 4, 2:21 PM · Restricted Project
reames accepted D103362: [LoopUnroll] Separate peeling from unrolling.

LGTM

Fri, Jun 4, 12:27 PM · Restricted Project
reames added a comment to D103362: [LoopUnroll] Separate peeling from unrolling.

@nikic I found your last comment convincing.

Fri, Jun 4, 12:25 PM · Restricted Project
reames added a comment to D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit.

We will be happy to test this on both our LE and BE systems. @stefanp do you have time to test this out or should I do it?

Seems like overkill for this one given the change is trivial and the test is target independent.

Fri, Jun 4, 9:26 AM · Restricted Project
reames updated the diff for D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit.

Fix silly typo in test.

Fri, Jun 4, 9:19 AM · Restricted Project
reames added inline comments to D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit.
Fri, Jun 4, 9:18 AM · Restricted Project
reames requested review of D103700: [LV] Fix bug when unrolling (only) a loop with non-latch exit.
Fri, Jun 4, 8:50 AM · Restricted Project

Thu, Jun 3

reames updated the diff for D103118: [SCEV] Compute exit counts for unsigned IVs using mustprogress semantics.

Address reviewer suggestion.

Thu, Jun 3, 2:46 PM · Restricted Project