Page MenuHomePhabricator

loladiro (Keno Fischer)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 22 2013, 10:24 PM (287 w, 1 d)

Recent Activity

Wed, May 8

loladiro added a comment to D50167: RFC: [SCEV] Add explicit representations of umin/smin.

Looked straightforward enough. Should be fixed by rPLO360238. I don't usually work on polly, so let me know if I misunderstood something.

Wed, May 8, 3:36 AM · Restricted Project
loladiro committed rGaa1b6f1cfb35: [polly][SCEV] Expand SCEV matcher cases for new smin/umin ops (authored by loladiro).
[polly][SCEV] Expand SCEV matcher cases for new smin/umin ops
Wed, May 8, 3:34 AM

Tue, May 7

loladiro committed rGa1a4adf4b919: [SCEV] Add explicit representations of umin/smin (authored by loladiro).
[SCEV] Add explicit representations of umin/smin
Tue, May 7, 8:27 AM

Fri, May 3

loladiro added a comment to D59729: [GVN] non-functional code movement.

@vtjnash Was the review comment addressed? If so I assume you want me to land this, since you don't have commit?

Fri, May 3, 5:53 AM · Restricted Project
loladiro added a comment to D50167: RFC: [SCEV] Add explicit representations of umin/smin.

@sanjoy Could you take another look to make sure I have addressed your comments to your satisfaction before I commit this?

Fri, May 3, 5:42 AM · Restricted Project
loladiro updated the diff for D50167: RFC: [SCEV] Add explicit representations of umin/smin.

A few minor tweaks.

Fri, May 3, 5:20 AM · Restricted Project
loladiro updated the diff for D50167: RFC: [SCEV] Add explicit representations of umin/smin.

Rebase and address review comments.

Fri, May 3, 4:40 AM · Restricted Project

Wed, May 1

loladiro committed rGa3e4b3bd3320: [SCEV] Use isKnownViaNonRecursiveReasoning for smax simplification (authored by loladiro).
[SCEV] Use isKnownViaNonRecursiveReasoning for smax simplification
Wed, May 1, 8:59 AM
loladiro added inline comments to D50167: RFC: [SCEV] Add explicit representations of umin/smin.
Wed, May 1, 8:35 AM · Restricted Project
loladiro added a comment to D61166: [SCEV] Use isKnownViaNonRecursiveReasoning for smax simplification.

and as I found out also stack overflows

For posterity, do you have some example IR and/or stack traces that you could attach to the commit message, or make a test case out of?

Wed, May 1, 8:18 AM · Restricted Project
loladiro committed rGd8f856d26549: [LoopInfo] Faster implementation of setLoopID. NFC. (authored by loladiro).
[LoopInfo] Faster implementation of setLoopID. NFC.
Wed, May 1, 7:39 AM

Fri, Apr 26

loladiro added a comment to D44650: Fix build of llvm-cfi-verify on mingw32.

Shouldn't instead LLVMCFIVerify be declared using add_llvm_library with the proper dependency on the add_llvm_tool line. It seems like the problem might just be mixing the llvm and the cmake variant of these commands.

Fri, Apr 26, 3:56 PM · Restricted Project
loladiro created D61215: [LoopInfo] Faster implementation of setLoopID. NFC..
Fri, Apr 26, 3:43 PM · Restricted Project
loladiro added a comment to D46460: [LoopInfo] Don't bail out early in getLoopID.

Actually, looks like the change to setLoopID is still relevant. I'll submit a new revision with just that change.

Fri, Apr 26, 3:20 PM · Restricted Project
loladiro abandoned D46460: [LoopInfo] Don't bail out early in getLoopID.

Looks like this got fixed by @jdoerfert in rL341926 with a near identical patch and test. No rebase required.

Fri, Apr 26, 3:14 PM · Restricted Project

Apr 25 2019

loladiro added a comment to D50167: RFC: [SCEV] Add explicit representations of umin/smin.

Alright, I have rebased this revision. I'd be happy to make the improvement to getAddExpr requested by @tvikram, since that seems independently useful, but I'd like to come to an agreement with @sanjoy and @mkazantsev on direction first.

Apr 25 2019, 6:21 PM · Restricted Project
loladiro created D61166: [SCEV] Use isKnownViaNonRecursiveReasoning for smax simplification.
Apr 25 2019, 6:14 PM · Restricted Project
loladiro updated the diff for D50167: RFC: [SCEV] Add explicit representations of umin/smin.

Undo a small spurious change I noticed while browsing the diff

Apr 25 2019, 5:59 PM · Restricted Project
loladiro updated the diff for D50167: RFC: [SCEV] Add explicit representations of umin/smin.

Rebased

Apr 25 2019, 5:50 PM · Restricted Project
loladiro committed rGe008be2b0723: [CMake][PowerPC] Recognize LLVM_NATIVE_TARGET="ppc64le" as PowerPC (authored by loladiro).
[CMake][PowerPC] Recognize LLVM_NATIVE_TARGET="ppc64le" as PowerPC
Apr 25 2019, 2:26 PM

Apr 4 2019

loladiro accepted D60231: Include invoke'd functions for recursive extract.
Apr 4 2019, 2:43 PM · Restricted Project

Apr 3 2019

loladiro added a comment to D60231: Include invoke'd functions for recursive extract.

Inline comment on implementation. The functionality looks fine to me.

Apr 3 2019, 1:35 PM · Restricted Project

Jan 23 2019

loladiro created D57118: [CMake][PowerPC] Recognize LLVM_NATIVE_TARGET="ppc64le" as PowerPC.
Jan 23 2019, 1:55 PM · Restricted Project

Sep 12 2018

loladiro updated the diff for D51842: [X86ISel] Implement byval lowering for Win64 calling convention.

Fix comment as requested by review, fix a small bug found in more extensive testing,
add a test case for the latter.

Sep 12 2018, 7:36 PM

Sep 9 2018

loladiro created D51842: [X86ISel] Implement byval lowering for Win64 calling convention.
Sep 9 2018, 2:01 PM

Aug 11 2018

loladiro updated the diff for D50167: RFC: [SCEV] Add explicit representations of umin/smin.

Fix ir names in tests broken by previous commits and proplerly match u/smin.

Aug 11 2018, 4:06 AM · Restricted Project
loladiro added a comment to D50167: RFC: [SCEV] Add explicit representations of umin/smin.

I have a general concern against such changes. In fact, you are introducing an alternative way to express the same thing. umin(a, b) and umax(~a, ~b) are the same, but now have 2 possible notations. It means that whatever pass or analysis that needs to recognize this pattern needs to be aware of both. Whenever the new node was not supported, it might be missed optimization opportunities. And we cannot know for sure how many such places there are, or will be. What motivation do you have for making this change? Is it strong enough to take a risk of missing optimization opportunities that I've just pointed out?

Aug 11 2018, 3:57 AM · Restricted Project

Aug 5 2018

loladiro updated the diff for D50167: RFC: [SCEV] Add explicit representations of umin/smin.

Fix a small bug discovered during production testing (
when expanding umin/umax, don't go to integers just because
types are unequal - the only reason to do is if one of the
operands in an integer and the other is not).

Aug 5 2018, 6:20 PM · Restricted Project

Aug 1 2018

loladiro created D50167: RFC: [SCEV] Add explicit representations of umin/smin.
Aug 1 2018, 6:06 PM · Restricted Project

Jul 30 2018

loladiro created D50010: [VNCoercion] Disallow coercion between different ni addrspaces.
Jul 30 2018, 2:09 PM

Jul 25 2018

loladiro updated the diff for D49832: [SCEV] Don't expand Wrap predicate using inttoptr in ni addrspaces.

Add a small fix found during testing that I forgot to commit.

Jul 25 2018, 7:29 PM
loladiro created D49832: [SCEV] Don't expand Wrap predicate using inttoptr in ni addrspaces.
Jul 25 2018, 7:19 PM

May 22 2018

loladiro added a comment to D46789: [StackProtector] Don't crash when passing post-SP IR back in.

If my memory recovey is successful and I understand the problem correctly, the best solution to me is to separate the mutation and analysis code in StackProtector. Suppose we split it into two passes "StackProtectorAnalysis" and "InsertStackProtectorPrologue", will it solve the problem? During the re-run, we only run StackProtectorAnalysis, but never again insert prologue. StackProtectorAnalysis should be kept as-is, without more special logic to detect whether the prologue is already generated (as this patch does). What do you think?

May 22 2018, 12:12 PM

May 17 2018

loladiro closed D41960: [Sink] Really really fix predicate in legality check.

This was merged in rL322311, closing to get it off my dashboard. In the future, ideally add

Differential Revision: https://reviews.llvm.org/D41960

with that syntax to the commit message, so Phabricator can close the revision automatically.

May 17 2018, 9:48 PM

May 16 2018

loladiro updated the diff for D46797: [X86DomainReassignment] Don't delete IMPLICIT_DEF nodes.

Remove incorrect comment from test case.

May 16 2018, 5:59 PM
loladiro updated the diff for D46800: [X86DomainReassignment] Don't compare stack-allocated values by address.

Add LLVM_DUMP_METHOD to the added dump() method.

May 16 2018, 5:57 PM
loladiro added inline comments to D46800: [X86DomainReassignment] Don't compare stack-allocated values by address.
May 16 2018, 5:46 PM
loladiro added inline comments to D46797: [X86DomainReassignment] Don't delete IMPLICIT_DEF nodes.
May 16 2018, 5:45 PM

May 12 2018

loladiro created D46800: [X86DomainReassignment] Don't compare stack-allocated values by address.
May 12 2018, 6:32 PM
loladiro created D46797: [X86DomainReassignment] Don't delete IMPLICIT_DEF nodes.
May 12 2018, 4:36 PM
loladiro updated the diff for D46787: [CommandLine] Error message for incorrect PositionalEatArgs usage.

Add test

May 12 2018, 3:56 PM
loladiro updated the diff for D46460: [LoopInfo] Don't bail out early in getLoopID.

Updated to use getLoopLatches and add a test.

May 12 2018, 3:14 PM · Restricted Project
loladiro added a comment to D46790: [bugpoint] Actually skip a modules that cause verifier errors.

Yeah, I've been thinking about what the best way to test this is. It seems somewhat hard to reliably trigger
a verifier fault by removing an instruction - llvm.stackprotector is the one I see most commonly, but I don't
really want to make the test depend on that behavior. One option is to teach bugpoint to use a custom verifier
pass and then write a custom verifier pass that e.g. checks for the presence of a call. That seems like a decently
attractive option, because it would be a useful feature in its own right. Some frontends have their own invariants
and corresponding verifier passes that they want to maintain in their early passes. Would be nice if bugpoint could
support that. The only thing I'm not quite sure off is to how to do this nicely. Presumably, we could use the analysis
name as a pass and run that through the pass manager. I'm just not sure how to get the analysis result out of that.

May 12 2018, 1:22 PM

May 11 2018

loladiro created D46790: [bugpoint] Actually skip a modules that cause verifier errors.
May 11 2018, 6:54 PM
loladiro created D46789: [StackProtector] Don't crash when passing post-SP IR back in.
May 11 2018, 6:40 PM
loladiro created D46787: [CommandLine] Error message for incorrect PositionalEatArgs usage.
May 11 2018, 5:06 PM

May 4 2018

loladiro created D46460: [LoopInfo] Don't bail out early in getLoopID.
May 4 2018, 1:49 PM · Restricted Project

Jan 31 2018

loladiro added inline comments to D42260: [JumpThreading] Don't select an edge that we know we can't thread.
Jan 31 2018, 2:22 PM
loladiro added a comment to D42262: [JumpThreading] Don't restrict cast-traversal to i1.

This happens fairly frequently for us in julia, because we use i8 to represent tags for tagged unions and then switch on them to discover which of the union components is active.
Without this patch I saw ~3x performance regressions in the worst benchmark (because it prevented other optimizations from seeing optimization opportunities based on the
active union component). I did not see compilation performance impact in my testing.

Jan 31 2018, 1:21 PM

Jan 18 2018

loladiro created D42262: [JumpThreading] Don't restrict cast-traversal to i1.
Jan 18 2018, 1:09 PM
loladiro created D42260: [JumpThreading] Don't select an edge that we know we can't thread.
Jan 18 2018, 12:38 PM

Jan 11 2018

loladiro accepted D41960: [Sink] Really really fix predicate in legality check.

Looks good to me with the minor tweak to the test. To keep the breadcrumb for future reference, I had changed this in D33179.

Jan 11 2018, 1:04 PM

Oct 26 2017

loladiro created D39336: [dsymutil] Check AttrInfo.Name validity before using it.
Oct 26 2017, 9:54 AM

Oct 25 2017

loladiro added a comment to D39297: [DynamicLibrary] Fix build on musl libc.
error: cannot initialize return object of type 'void *' with an rvalue of type 'FILE *const *' (aka '_IO_FILE *const *')
    EXPLICIT_SYMBOL(stderr);
    ^~~~~~~~~~~~~~~~~~~~~~~
Oct 25 2017, 12:05 PM
loladiro added reviewers for D39297: [DynamicLibrary] Fix build on musl libc: krytarowski, dim.
Oct 25 2017, 11:56 AM
loladiro created D39297: [DynamicLibrary] Fix build on musl libc.
Oct 25 2017, 9:53 AM

Sep 25 2017

loladiro created D38264: [docs] Fix cross-compile instructions.
Sep 25 2017, 3:22 PM

Sep 19 2017

loladiro added a comment to D37939: [Mem2Reg] Also handle memcpy.

this is the julia pass pipeline (https://github.com/JuliaLang/julia/blob/master/src/jitlayers.cpp#L148). IIRC the original list of passes came from VMKit,
but the pass list was adjusted as needed over the years.

Sep 19 2017, 10:45 AM

Sep 17 2017

loladiro updated the diff for D37939: [Mem2Reg] Also handle memcpy.

Fix a small bug and also look through a single level of bitcasts.
Since IRBuilder automatically inserts bitcasts to i8*, it seems
prudent to handle that case.

Sep 17 2017, 7:30 PM

Sep 15 2017

loladiro created D37939: [Mem2Reg] Also handle memcpy.
Sep 15 2017, 3:39 PM

Jul 7 2017

loladiro accepted D35106: [cloning] Do not duplicate types when cloning functions.

The implementation of this looks good to me. I guess I'm not entirely sure why it's necessary for that type to be distinct in the first place, but that's probably a different discussion from just fixing this bug.

Jul 7 2017, 12:44 AM

Jun 29 2017

loladiro added inline comments to D34335: Fix invalid ptrtoint in InstCombine.
Jun 29 2017, 12:33 PM

Jun 28 2017

loladiro updated the diff for D32139: [AliasSetTracker] Don't drop AA MD so eagerly.

Factor out intersection code into a method of AAMDNodes

Jun 28 2017, 4:03 PM

Jun 20 2017

loladiro added a comment to D32139: [AliasSetTracker] Don't drop AA MD so eagerly.

Bump

Jun 20 2017, 3:55 PM

Jun 13 2017

loladiro added a comment to D31954: [InstCombine] Retain TBAA when narrowing loads.

Bump.

Jun 13 2017, 2:07 PM

Jun 10 2017

loladiro added a comment to D31924: SROA: Allow eliminating addrspacecasted allocas.

Having pondered this some more, I wonder if what we're missing is an annotation on the addrspace cast itself that indicates whether or not GEPs may be commuted past it (could call it inbounds or something else). It seems like in many cases (including allocas). The frontend (or whoever else is doing language/target specific work) can often know whether the entire object is available in the target address space (e.g. because the entire stack always is).

Jun 10 2017, 3:57 PM
loladiro accepted D32203: [SROA] Add support for non-integral pointers.

LGTM, FWIW. I encountered this issue as well and this seems like the correct solution.

Jun 10 2017, 3:10 PM

Jun 9 2017

loladiro added a comment to D33110: [CodeGenPrepare] Don't create inttoptr for ni ptrs.

Bump on review for this.

Jun 9 2017, 12:33 PM
loladiro added a comment to D33179: [Sink] Fix predicate in legality check.

I'm gonna go ahead and commit this anyway, since this fixes a miscompile. If there are any comments I'll take them in post-commit review.

Jun 9 2017, 12:14 PM

Jun 2 2017

loladiro added a comment to D33110: [CodeGenPrepare] Don't create inttoptr for ni ptrs.

Bump. @sanjoy does this look good to you?

Jun 2 2017, 12:11 PM
loladiro added a comment to D33179: [Sink] Fix predicate in legality check.

Bump. Would be great if somebody could review this. I know Sink it's not really actively developed, but I think this is a pretty straightforward fix.

Jun 2 2017, 12:10 PM

Jun 1 2017

loladiro added a comment to D32593: [SROA] Fix crash due to bad bitcast.

Sure, see if you think this is the right way to do it. If so, mark it as accepted, and I'll go ahead and commit it. I feel like we've given other people enough time to weigh in ;).

Jun 1 2017, 2:06 PM
loladiro added a comment to D33705: [CGVTables] Finalize SP before attempting to clone it.

I don't think that change is entirely necessary. I don't have any strong objections to it, but I also don't see a good reason to require it. In any case, let me get this in to be able to re-land D33655 and we can revisit the more disruptive change later.

Jun 1 2017, 1:36 PM
loladiro added a comment to D33705: [CGVTables] Finalize SP before attempting to clone it.

There's already such a test case, but the cloning currently doesn't assert properly (but it can generate incorrect code). D33655 fixes that up, so I think the testing is covered once that LLVM commit goes in. I'll hold off a little while to give @aprantl a chance to look at this as well, but I do want to get this in in short order, so I can recommit D33655.

Jun 1 2017, 1:16 PM
loladiro added a comment to D33705: [CGVTables] Finalize SP before attempting to clone it.

@aprantl @dblaikie See if you like this better.

Jun 1 2017, 1:08 PM
loladiro updated the diff for D33705: [CGVTables] Finalize SP before attempting to clone it.

Finalize all subprograms when we're done emitting them.
The one we're interested in is a side effect, but doing
this uniformly might be cleaner and help avoid similar errors in the future.

Jun 1 2017, 1:07 PM
loladiro added a comment to D24371: Add diagnostics to require_constant_initialization.

Sure, I'll commit it.

Jun 1 2017, 10:52 AM

May 31 2017

loladiro updated the diff for D22499: [llvm-config] Report --bindir based on LLVM_TOOLS_INSTALL_DIR.

Handle absolute path case

May 31 2017, 3:30 PM
loladiro added a comment to D22499: [llvm-config] Report --bindir based on LLVM_TOOLS_INSTALL_DIR.

Not sure what you're asking. llvm-config is certainly generally installed there, but --bindir is wrong without this patch, so people using that will be confused (at least that was the case when I wrote this patch - it's been a while).

May 31 2017, 11:42 AM
loladiro updated the diff for D33704: [DIBuilder] Add a more fine-grained finalization method.

I don't think there's really a natural place in DIBuilder to do this, but
I suspect that most frontends would have one.

May 31 2017, 10:51 AM
loladiro added a comment to D33704: [DIBuilder] Add a more fine-grained finalization method.

Will you only invoke it on functions to be cloned in clang or on all subprograms?

May 31 2017, 9:06 AM

May 30 2017

loladiro created D33705: [CGVTables] Finalize SP before attempting to clone it.
May 30 2017, 5:30 PM
loladiro created D33704: [DIBuilder] Add a more fine-grained finalization method.
May 30 2017, 5:26 PM
loladiro added a comment to D33655: [Cloning] Take another pass at properly cloning debug info.

FYI, reverted due to buildbot failure. Will investigate later today.

May 30 2017, 11:57 AM

May 29 2017

loladiro added a comment to D33655: [Cloning] Take another pass at properly cloning debug info.

Well, note really, but that's sort of separate from this revision. A more accurate (though IMO still slightly confusing) description is the one that's on the flag:

/// If this flag is set, the remapper knows that only local values within a
/// function (such as an instruction or argument) are mapped, not global
/// values like functions and global metadata.
RF_NoModuleLevelChanges = 1,
May 29 2017, 2:02 PM
loladiro created D33655: [Cloning] Take another pass at properly cloning debug info.
May 29 2017, 12:23 PM

May 28 2017

loladiro added a comment to D32975: Make it illegal for two Functions to point to the same DISubprogram.

I'm really confused by the entirety of the changes to the CloneFunction parts of this. The whole point of the ValueMapper is to fix up value/metadata references. Why do we need a manual version? Also what's up with the DISubprogram::getDistinct? The ValueMapper will create new distinct nodes as it goes along unless RF_MoveDistinctMDs is set (which it isn't inside CloneFunction). Could the problem perhaps have been an incorrectly unset ModuleLevelChanges flag? I'm seeing a ton of assertion failures after this change. I'll submit an appropriate PR to revert the CloneFunction parts of this.

May 28 2017, 4:00 PM

May 27 2017

loladiro added a comment to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.

Thanks! I'm a little confused why the compiler is complaining there - I see this pattern used all over the place for 1-element array construction, but your fix is of course fine.

May 27 2017, 4:30 PM

May 22 2017

loladiro updated the diff for D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.

Add a C++ test to make sure the SCEV code path is tested

May 22 2017, 1:39 PM

May 20 2017

loladiro added inline comments to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.
May 20 2017, 9:31 PM

May 19 2017

loladiro added a comment to D32593: [SROA] Fix crash due to bad bitcast.

Bump. Can somebody review this, so we can get this in. @arsenm maybe?

May 19 2017, 3:18 PM
loladiro added a comment to D33361: [InstCombine] Fix inbounds gep for addrspacecasts.

Please see the discussion in D31924 - it's not clear that instcombine is allowed to introduce addrspacecasts that weren't in the original program. I know there's a couple cases like that in the code base already, but no need to add more.

May 19 2017, 10:00 AM

May 17 2017

loladiro updated the diff for D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.

Changed the patch to do the argument re-arranging in the caller instead. I think
that's more localized to just this non-integral pointer change.

May 17 2017, 4:16 AM

May 16 2017

loladiro added inline comments to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.
May 16 2017, 4:32 PM
loladiro added inline comments to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.
May 16 2017, 2:55 PM
loladiro added inline comments to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.
May 16 2017, 1:42 PM

May 15 2017

loladiro added a comment to D31924: SROA: Allow eliminating addrspacecasted allocas.

Do you mean non-integral pointers?

May 15 2017, 6:58 AM

May 14 2017

loladiro created D33179: [Sink] Fix predicate in legality check.
May 14 2017, 9:28 PM
loladiro added a comment to D31924: SROA: Allow eliminating addrspacecasted allocas.

Was there a decision reached here on what the correct semantics are? There are other places in LLVM (I found one in instcombine - there may be others) which do make the assumption that this change is proposing to introduce to the langref. Personally, I don't think this transformation should be allowed. I know there are architectures where different address spaces have different GEP behavior (though I'm not sure if this is the case for any in-tree backend). Nevertheless, if people do feel like this should be allowed (e.g. because such architectures should use something other than addrspacecast to convert between such address spaces), that's fine with me as well, but I think there should a clear statement in the langref on way or the other. As is, different people read the langref differently.

May 14 2017, 3:20 PM

May 12 2017

loladiro created D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.
May 12 2017, 6:56 AM