loladiro (Keno Fischer)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 22 2013, 10:24 PM (251 w, 6 d)

Recent Activity

Wed, Sep 12

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.

Wed, Sep 12, 7:36 PM

Sun, Sep 9

loladiro created D51842: [X86ISel] Implement byval lowering for Win64 calling convention.
Sun, Sep 9, 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
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

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

Aug 1 2018

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

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
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

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

May 11 2017

loladiro created D33110: [CodeGenPrepare] Don't create inttoptr for ni ptrs.
May 11 2017, 4:10 PM

May 9 2017

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

Bump. Does this look good?

May 9 2017, 2:23 PM

Apr 27 2017

loladiro created D32623: [GVN] Fix a crash on encountering non-integral pointers.
Apr 27 2017, 4:10 PM
loladiro updated the diff for D32593: [SROA] Fix crash due to bad bitcast.

Add some CHECK lines to the test case

Apr 27 2017, 12:20 PM
loladiro created D32593: [SROA] Fix crash due to bad bitcast.
Apr 27 2017, 6:51 AM

Apr 26 2017

loladiro added inline comments to D32540: [MVT] add v1i1 MVT.
Apr 26 2017, 8:30 AM

Apr 17 2017

loladiro updated the diff for D31954: [InstCombine] Retain TBAA when narrowing loads.

Run clang-format over the diff

Apr 17 2017, 2:50 PM
loladiro updated the diff for D31954: [InstCombine] Retain TBAA when narrowing loads.

Now with testcases for the extra cases I discovered.

Apr 17 2017, 2:50 PM
loladiro created D32139: [AliasSetTracker] Don't drop AA MD so eagerly.
Apr 17 2017, 2:32 PM

Apr 12 2017

loladiro updated the diff for D31954: [InstCombine] Retain TBAA when narrowing loads.

There were a couple more of instances of the same pattern in this code.
I'll write some tests for all these cases next week (famous last words).

Apr 12 2017, 9:43 AM

Apr 11 2017

loladiro created D31954: [InstCombine] Retain TBAA when narrowing loads.
Apr 11 2017, 12:21 PM

Apr 10 2017

loladiro updated the diff for D31904: [StripDeadDebug/DIFinder] Track inlined SPs.

Updated to include the test for DIFinder

Apr 10 2017, 1:18 PM
loladiro created D31904: [StripDeadDebug/DIFinder] Track inlined SPs.
Apr 10 2017, 1:05 PM

Apr 6 2017

loladiro added a comment to D31509: [InstCombine] Combine vector shuffles if the same operand can be reused.

I wasn't suggesting we blindly trust the user on shuffles, but that we somehow distinguish between masks generated by some earlier LLVM pass and masks passed in by the user. For the former I feel like it would make a lot of sense to be more aggressive about folding them than we are now, which for the latter the user may have used some target specific knowledge, so we may want to be less aggressive. I'm also fine with a TTI hook, happy to write the code. Somebody has to make a call here, and I don't think I'm the best person to do so - I just want all those redundant shuffles out of my code ;).

Apr 6 2017, 8:56 PM
loladiro added inline comments to D31722: [llvm-extract] Add option for recursive extraction.
Apr 6 2017, 1:02 PM
loladiro updated the diff for D31722: [llvm-extract] Add option for recursive extraction.

Address review comments

Apr 6 2017, 1:00 PM
loladiro added a comment to D18738: Add new !unconditionally_dereferenceable load instruction metadata.

I'm a bit late to the party here, but I'm facing similar problems, so I'm interested in a clean solution. I wonder though if what we want to express isn't some sort of "type-based dereferencability annotations". For example the semantics I care about are essentially, "if you know you have a defereferencable pointer, you can go and dereference any other valid (managed) pointers the pointed to data references (recursively)". This has to be true for me, because the GC walks all pointers recursively that way. Of course the problem with this is that the compiler doesn't know which part of the referenced data are valid pointers for this purpose (and it can't just be based on the struct type, because we do store pointers to unmanaged data). So if we had a way to express to the compiler "here are the pointers in this struct that you may assume dereferencable", that would work very well for me. Would that solve your problem as well?

Apr 6 2017, 7:50 AM

Apr 5 2017

loladiro created D31722: [llvm-extract] Add option for recursive extraction.
Apr 5 2017, 1:22 PM
loladiro created D31720: [StripDeadDebugInfo] Drop dead CUs entirely.
Apr 5 2017, 12:11 PM

Apr 4 2017

loladiro added a comment to D31509: [InstCombine] Combine vector shuffles if the same operand can be reused.

If a TTI hook is not the right way to go here, maybe we can find a way to actually express what we want here, maybe via metadata or some other specification on shufflevector that would tell the optimizer not to touch the shufflemask unless it's confident it can improve upon it. As it is, I see my CPU with a perfectly good shuffle instruction shuffle the same vector three times, because LLVM refuses to combine these shuffles - pretty embarrassing ;).

Apr 4 2017, 5:26 PM
loladiro added a comment to D31509: [InstCombine] Combine vector shuffles if the same operand can be reused.

Hmm, unless I did something wrong, the backend generates equivalent code whether or not we merge the shuffles. Or did you mean that we could do a better job with the three IR instruction form even though we currently don't?

Apr 4 2017, 5:12 PM
loladiro added a comment to D28759: [ExecutionDepsFix] Improve clearance calculation for loops.

Can you try https://reviews.llvm.org/D31681? I wasn't able to reproduce the problem with your example, it kept crashing other parts of the compiler ;).

Apr 4 2017, 3:19 PM
loladiro created D31681: [ExecutionDepsFix] Don't recurse over the CFG.
Apr 4 2017, 3:18 PM