[llvm] r267232 - Revert r267210, it makes clang assert (PR27490).
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 22 15:37:44 PDT 2016
Also, I'm assuming this is https://llvm.org/bugs/show_bug.cgi?id=27490?
If so, could you please attach an IR reproducer? I appreciate the C
reproducer, but I do not have a recent build of clang. I work only on
LLVM.
Philip
On 04/22/2016 03:35 PM, Philip Reames via llvm-commits wrote:
> Nico,
>
> I have no objection to this being reverted, but in the future, please
> make sure to reply to the original commit thread so that the author
> knows something is going on. There was a discussion on IRC, but I got
> drug into that quite late and email is our system of record.
>
> Philip
>
> On 04/22/2016 03:08 PM, Nico Weber via llvm-commits wrote:
>> Author: nico
>> Date: Fri Apr 22 17:08:42 2016
>> New Revision: 267232
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=267232&view=rev
>> Log:
>> Revert r267210, it makes clang assert (PR27490).
>>
>> Modified:
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> llvm/trunk/test/Transforms/InstCombine/atomic.ll
>>
>> Modified:
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=267232&r1=267231&r2=267232&view=diff
>> ==============================================================================
>>
>> ---
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> (original)
>> +++
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> Fri Apr 22 17:08:42 2016
>> @@ -327,8 +327,6 @@ static LoadInst *combineLoadToNewType(In
>> LoadInst *NewLoad = IC.Builder->CreateAlignedLoad(
>> IC.Builder->CreateBitCast(Ptr, NewTy->getPointerTo(AS)),
>> LI.getAlignment(), LI.getName() + Suffix);
>> - NewLoad->setAtomic(LI.getOrdering(), LI.getSynchScope());
>> - assert(!LI.isVolatile() && "volatile unhandled here");
>> MDBuilder MDB(NewLoad->getContext());
>> for (const auto &MDPair : MD) {
>> unsigned ID = MDPair.first;
>> @@ -401,8 +399,6 @@ static StoreInst *combineStoreToNewValue
>> StoreInst *NewStore = IC.Builder->CreateAlignedStore(
>> V, IC.Builder->CreateBitCast(Ptr,
>> V->getType()->getPointerTo(AS)),
>> SI.getAlignment());
>> - NewStore->setAtomic(SI.getOrdering(), SI.getSynchScope());
>> - assert(!SI.isVolatile() && "volatile unhandled here");
>> for (const auto &MDPair : MD) {
>> unsigned ID = MDPair.first;
>> MDNode *N = MDPair.second;
>> @@ -460,9 +456,9 @@ static StoreInst *combineStoreToNewValue
>> /// later. However, it is risky in case some backend or other part
>> of LLVM is
>> /// relying on the exact type loaded to select appropriate atomic
>> operations.
>> static Instruction *combineLoadToOperationType(InstCombiner &IC,
>> LoadInst &LI) {
>> - // FIXME: We could probably with some care handle both volatile
>> and ordered
>> - // atomic loads here but it isn't clear that this is important.
>> - if (!LI.isUnordered())
>> + // FIXME: We could probably with some care handle both volatile
>> and atomic
>> + // loads here but it isn't clear that this is important.
>> + if (!LI.isSimple())
>> return nullptr;
>> if (LI.use_empty())
>> @@ -896,7 +892,6 @@ Instruction *InstCombiner::visitLoadInst
>> V1->setAtomic(LI.getOrdering(), LI.getSynchScope());
>> V2->setAlignment(Align);
>> V2->setAtomic(LI.getOrdering(), LI.getSynchScope());
>> - assert(!LI.isVolatile() && "volatile unhandled here");
>> return SelectInst::Create(SI->getCondition(), V1, V2);
>> }
>> @@ -939,9 +934,9 @@ Instruction *InstCombiner::visitLoadInst
>> /// the store instruction as otherwise there is no way to signal
>> whether it was
>> /// combined or not: IC.EraseInstFromFunction returns a null pointer.
>> static bool combineStoreToValueType(InstCombiner &IC, StoreInst &SI) {
>> - // FIXME: We could probably with some care handle both volatile
>> and ordered
>> - // atomic stores here but it isn't clear that this is important.
>> - if (!SI.isUnordered())
>> + // FIXME: We could probably with some care handle both volatile
>> and atomic
>> + // stores here but it isn't clear that this is important.
>> + if (!SI.isSimple())
>> return false;
>> Value *V = SI.getValueOperand();
>>
>> Modified: llvm/trunk/test/Transforms/InstCombine/atomic.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/atomic.ll?rev=267232&r1=267231&r2=267232&view=diff
>> ==============================================================================
>>
>> --- llvm/trunk/test/Transforms/InstCombine/atomic.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/atomic.ll Fri Apr 22
>> 17:08:42 2016
>> @@ -173,45 +173,6 @@ define i32 @test17(i1 %cnd) {
>> ret i32 %x
>> }
>> -declare void @clobber()
>> -
>> -define i32 @test18(float* %p) {
>> -; CHECK-LABEL: define i32 @test18(
>> -; CHECK: load atomic i32, i32* [[A:%.*]] unordered, align 4
>> -; CHECK: store atomic i32 [[B:%.*]], i32* [[C:%.*]] unordered, align 4
>> - %x = load atomic float, float* %p unordered, align 4
>> - call void @clobber() ;; keep the load around
>> - store atomic float %x, float* %p unordered, align 4
>> - ret i32 0
>> -}
>> -
>> -; TODO: probably also legal in this case
>> -define i32 @test19(float* %p) {
>> -; CHECK-LABEL: define i32 @test19(
>> -; CHECK: load atomic float, float* %p seq_cst, align 4
>> -; CHECK: store atomic float %x, float* %p seq_cst, align 4
>> - %x = load atomic float, float* %p seq_cst, align 4
>> - call void @clobber() ;; keep the load around
>> - store atomic float %x, float* %p seq_cst, align 4
>> - ret i32 0
>> -}
>> -
>> -define i32 @test20(i32** %p, i8* %v) {
>> -; CHECK-LABEL: define i32 @test20(
>> -; CHECK: store atomic i8* %v, i8** [[D:%.*]] unordered, align 4
>> - %cast = bitcast i8* %v to i32*
>> - store atomic i32* %cast, i32** %p unordered, align 4
>> - ret i32 0
>> -}
>> -; TODO: probably also legal in this case
>> -define i32 @test21(i32** %p, i8* %v) {
>> -; CHECK-LABEL: define i32 @test21(
>> -; CHECK: store atomic i32* %cast, i32** %p monotonic, align 4
>> - %cast = bitcast i8* %v to i32*
>> - store atomic i32* %cast, i32** %p monotonic, align 4
>> - ret i32 0
>> -}
>> -
>> define i32 @test22(i1 %cnd) {
>> ; CHECK-LABEL: define i32 @test22(
>> ; CHECK: [[PHI:%.*]] = phi i32
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list