Page MenuHomePhabricator

[Attributor][Fix] Initialize the cache prior to using it
AcceptedPublic

Authored by jdoerfert on Thu, Sep 5, 10:21 AM.

Details

Reviewers
uenoku
sstefan1
Summary

There were segfaults as we modified and iterated the instruction maps in
the cache at the same time. This was happening because we created new
instructions while we populated the cache. This fix changes the order
in which we perform these actions. First, the caches for the whole
module are created, then we start to create abstract attributes.

I don't have a unit test but the LLVM test suite exposes this problem.

Event Timeline

jdoerfert created this revision.Thu, Sep 5, 10:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Sep 5, 10:21 AM
jdoerfert updated this revision to Diff 219007.Thu, Sep 5, 5:05 PM

Update tests

xbolva00 added inline comments.
llvm/lib/Transforms/IPO/Attributor.cpp
3051

Check ´LivenessAA´ before loop

sstefan1 added inline comments.Fri, Sep 6, 12:04 AM
llvm/lib/Transforms/IPO/Attributor.cpp
3051

I don't think that is practical. If LivenessAA is not null, we would still need a way to indicate it can be dereferenced. Pred should be executed even if LivenessAA is null.

jdoerfert marked an inline comment as done.Sat, Sep 7, 6:16 PM
jdoerfert added inline comments.
llvm/lib/Transforms/IPO/Attributor.cpp
3051

I would need to duplicate the loop (as @sstefan1 noted) which is a compiler task ;)

sstefan1 accepted this revision.Wed, Sep 11, 1:38 PM
This revision is now accepted and ready to land.Wed, Sep 11, 1:38 PM