[llvm] f08148e - [ConstProp] Fix folding of pointer icmp with signed predicates
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 8 09:27:06 PST 2021
On 3/8/21 9:25 AM, Philip Reames via llvm-commits wrote:
> Nikita,
>
> At least on common platforms, use programs can't place globals in the
> top half of the address space. Generally, the high bit being one
> means "kernel space".
possible confusing typo: "user programs" not "use programs"
>
> We probably shouldn't bake that assumption in outright - otherwise,
> compiling the kernel becomes "fun" - but it'd be nice not to loose it
> entirely. Is there a configuration flag we can base this off?
>
> Philip
>
> On 3/8/21 8:14 AM, Nikita Popov via llvm-commits wrote:
>> Author: Nikita Popov
>> Date: 2021-03-08T17:12:12+01:00
>> New Revision: f08148e874088a07b972203a183db00de9c38a70
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/f08148e874088a07b972203a183db00de9c38a70
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/f08148e874088a07b972203a183db00de9c38a70.diff
>>
>> LOG: [ConstProp] Fix folding of pointer icmp with signed predicates
>>
>> While @g ugt null is always true (ignoring weak symbols),
>> @g sgt null is not necessarily the case -- that would imply that
>> it is forbidden to place globals in the high half of the address
>> space.
>>
>> Added:
>>
>> Modified:
>> llvm/lib/IR/ConstantFold.cpp
>> llvm/test/Transforms/InstSimplify/ConstProp/icmp-null.ll
>>
>> Removed:
>>
>>
>> ################################################################################
>>
>> diff --git a/llvm/lib/IR/ConstantFold.cpp
>> b/llvm/lib/IR/ConstantFold.cpp
>> index 3903d72ac7c3..2e0ea4c50e79 100644
>> --- a/llvm/lib/IR/ConstantFold.cpp
>> +++ b/llvm/lib/IR/ConstantFold.cpp
>> @@ -1838,35 +1838,27 @@ static ICmpInst::Predicate
>> evaluateICmpRelation(Constant *V1, Constant *V2,
>> // If we are comparing a GEP to a null pointer, check to
>> see if the base
>> // of the GEP equals the null pointer.
>> if (const GlobalValue *GV = dyn_cast<GlobalValue>(CE1Op0)) {
>> - if (GV->hasExternalWeakLinkage())
>> - // Weak linkage GVals could be zero or not. We're
>> comparing that
>> - // to null pointer so its greater-or-equal
>> - return isSigned ? ICmpInst::ICMP_SGE : ICmpInst::ICMP_UGE;
>> - else
>> - // If its not weak linkage, the GVal must have a
>> non-zero address
>> - // so the result is greater-than
>> - return isSigned ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT;
>> + // If its not weak linkage, the GVal must have a non-zero
>> address
>> + // so the result is greater-than
>> + if (!GV->hasExternalWeakLinkage())
>> + return ICmpInst::ICMP_UGT;
>> } else if (isa<ConstantPointerNull>(CE1Op0)) {
>> // If we are indexing from a null pointer, check to see
>> if we have any
>> // non-zero indices.
>> for (unsigned i = 1, e = CE1->getNumOperands(); i != e; ++i)
>> if (!CE1->getOperand(i)->isNullValue())
>> // Offsetting from null, must not be equal.
>> - return isSigned ? ICmpInst::ICMP_SGT :
>> ICmpInst::ICMP_UGT;
>> + return ICmpInst::ICMP_UGT;
>> // Only zero indexes from null, must still be zero.
>> return ICmpInst::ICMP_EQ;
>> }
>> // Otherwise, we can't really say if the first operand is
>> null or not.
>> } else if (const GlobalValue *GV2 = dyn_cast<GlobalValue>(V2)) {
>> if (isa<ConstantPointerNull>(CE1Op0)) {
>> - if (GV2->hasExternalWeakLinkage())
>> - // Weak linkage GVals could be zero or not. We're
>> comparing it to
>> - // a null pointer, so its less-or-equal
>> - return isSigned ? ICmpInst::ICMP_SLE : ICmpInst::ICMP_ULE;
>> - else
>> - // If its not weak linkage, the GVal must have a
>> non-zero address
>> - // so the result is less-than
>> - return isSigned ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_ULT;
>> + // If its not weak linkage, the GVal must have a non-zero
>> address
>> + // so the result is less-than
>> + if (!GV2->hasExternalWeakLinkage())
>> + return ICmpInst::ICMP_ULT;
>> } else if (const GlobalValue *GV =
>> dyn_cast<GlobalValue>(CE1Op0)) {
>> if (GV == GV2) {
>> // If this is a getelementptr of the same global, then
>> it must be
>> @@ -1876,7 +1868,7 @@ static ICmpInst::Predicate
>> evaluateICmpRelation(Constant *V1, Constant *V2,
>> assert(CE1->getNumOperands() == 2 &&
>> !CE1->getOperand(1)->isNullValue() &&
>> "Surprising getelementptr!");
>> - return isSigned ? ICmpInst::ICMP_SGT : ICmpInst::ICMP_UGT;
>> + return ICmpInst::ICMP_UGT;
>> } else {
>> if (CE1GEP->hasAllZeroIndices())
>> return areGlobalsPotentiallyEqual(GV, GV2);
>>
>> diff --git
>> a/llvm/test/Transforms/InstSimplify/ConstProp/icmp-null.ll
>> b/llvm/test/Transforms/InstSimplify/ConstProp/icmp-null.ll
>> index 46fd8c4bfce8..41a64205633e 100644
>> --- a/llvm/test/Transforms/InstSimplify/ConstProp/icmp-null.ll
>> +++ b/llvm/test/Transforms/InstSimplify/ConstProp/icmp-null.ll
>> @@ -110,7 +110,7 @@ define i1 @global_gep_ugt_null() {
>> define i1 @global_gep_sgt_null() {
>> ; CHECK-LABEL: @global_gep_sgt_null(
>> -; CHECK-NEXT: ret i1 true
>> +; CHECK-NEXT: ret i1 icmp sgt ([2 x i32]* getelementptr inbounds
>> ([2 x i32], [2 x i32]* @g, i64 1), [2 x i32]* null)
>> ;
>> %gep = getelementptr inbounds [2 x i32], [2 x i32]* @g, i64 1
>> %cmp = icmp sgt [2 x i32]* %gep, null
>> @@ -137,7 +137,7 @@ define i1 @null_gep_ugt_null() {
>> define i1 @null_gep_sgt_null() {
>> ; CHECK-LABEL: @null_gep_sgt_null(
>> -; CHECK-NEXT: ret i1 true
>> +; CHECK-NEXT: ret i1 icmp sgt (i8* getelementptr (i8, i8* null,
>> i64 ptrtoint (i32* @g2 to i64)), i8* null)
>> ;
>> %gep = getelementptr i8, i8* null, i64 ptrtoint (i32* @g2 to i64)
>> %cmp = icmp sgt i8* %gep, null
>> @@ -164,7 +164,7 @@ define i1 @null_gep_ult_global() {
>> define i1 @null_gep_slt_global() {
>> ; CHECK-LABEL: @null_gep_slt_global(
>> -; CHECK-NEXT: ret i1 true
>> +; CHECK-NEXT: ret i1 icmp slt ([2 x i32]* getelementptr ([2 x
>> i32], [2 x i32]* null, i64 ptrtoint (i32* @g2 to i64)), [2 x i32]* @g)
>> ;
>> %gep = getelementptr [2 x i32], [2 x i32]* null, i64 ptrtoint
>> (i32* @g2 to i64)
>> %cmp = icmp slt [2 x i32]* %gep, @g
>> @@ -191,7 +191,7 @@ define i1 @global_gep_ugt_global() {
>> define i1 @global_gep_sgt_global() {
>> ; CHECK-LABEL: @global_gep_sgt_global(
>> -; CHECK-NEXT: ret i1 true
>> +; CHECK-NEXT: ret i1 icmp sgt ([2 x i32]* getelementptr inbounds
>> ([2 x i32], [2 x i32]* @g, i64 1), [2 x i32]* @g)
>> ;
>> %gep = getelementptr inbounds [2 x i32], [2 x i32]* @g, i64 1
>> %cmp = icmp sgt [2 x i32]* %gep, @g
>> @@ -209,7 +209,7 @@ define i1 @global_gep_ugt_global_neg_offset() {
>> define i1 @global_gep_sgt_global_neg_offset() {
>> ; CHECK-LABEL: @global_gep_sgt_global_neg_offset(
>> -; CHECK-NEXT: ret i1 true
>> +; CHECK-NEXT: ret i1 icmp sgt ([2 x i32]* getelementptr ([2 x
>> i32], [2 x i32]* @g, i64 -1), [2 x i32]* @g)
>> ;
>> %gep = getelementptr [2 x i32], [2 x i32]* @g, i64 -1
>> %cmp = icmp sgt [2 x i32]* %gep, @g
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list