[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Nikita Popov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf89f7da999f3: [IR] Convert null-pointer-is-valid into an 
enum attribute (authored by nikic).

Changed prior to commit:
  https://reviews.llvm.org/D78862?vs=261375=264285#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/delete-null-pointer-checks.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/include/llvm/IR/AutoUpgrade.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Analysis/MemorySSA/cyclicphi.ll
  llvm/test/Analysis/ValueTracking/assume.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/Attributor/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Attributor/align.ll
  llvm/test/Transforms/Attributor/nocapture-1.ll
  llvm/test/Transforms/Attributor/nonnull.ll
  llvm/test/Transforms/Attributor/norecurse.ll
  llvm/test/Transforms/Attributor/undefined_behavior.ll
  llvm/test/Transforms/CorrelatedValuePropagation/non-null.ll
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/MallocSROA-section-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi.ll
  llvm/test/Transforms/GlobalOpt/load-store-global-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2.ll
  llvm/test/Transforms/GlobalOpt/storepointer-compare-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/storepointer-no-null-opt.ll
  llvm/test/Transforms/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Inline/attributes.ll
  llvm/test/Transforms/InstCombine/atomic.ll
  llvm/test/Transforms/InstCombine/invariant.group.ll
  llvm/test/Transforms/InstCombine/invoke.ll
  llvm/test/Transforms/InstCombine/lifetime-no-null-opt.ll
  llvm/test/Transforms/InstCombine/load.ll
  llvm/test/Transforms/InstCombine/mem-deref-bytes.ll
  llvm/test/Transforms/InstCombine/memchr.ll
  llvm/test/Transforms/InstCombine/memcpy-addrspace.ll
  llvm/test/Transforms/InstCombine/memcpy-from-global.ll
  llvm/test/Transforms/InstCombine/memrchr.ll
  llvm/test/Transforms/InstCombine/select.ll
  llvm/test/Transforms/InstCombine/store.ll
  llvm/test/Transforms/InstCombine/strchr-1.ll
  llvm/test/Transforms/InstCombine/strcpy_chk-64.ll
  llvm/test/Transforms/InstCombine/strlen-1.ll
  llvm/test/Transforms/InstCombine/strncat-2.ll
  llvm/test/Transforms/InstCombine/strncmp-1.ll
  llvm/test/Transforms/InstCombine/strrchr-1.ll
  llvm/test/Transforms/InstCombine/strstr-1.ll
  llvm/test/Transforms/InstCombine/wcslen-1.ll
  llvm/test/Transforms/InstSimplify/compare.ll
  llvm/test/Transforms/LoopIdiom/pr28196.ll
  llvm/test/Transforms/LoopVersioning/lcssa.ll
  llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
  llvm/test/Transforms/SimplifyCFG/invoke.ll
  llvm/test/Transforms/SimplifyCFG/phi-undef-loadstore.ll
  llvm/test/Transforms/SimplifyCFG/trap-no-null-opt-debugloc.ll
  llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
  llvm/test/Transforms/Util/assume-builder.ll
  mlir/test/Target/llvmir.mlir

Index: mlir/test/Target/llvmir.mlir
===
--- mlir/test/Target/llvmir.mlir
+++ mlir/test/Target/llvmir.mlir
@@ -1205,12 +1205,12 @@
 
 // CHECK-LABEL: @passthrough
 // CHECK: #[[ATTR_GROUP:[0-9]*]]
-llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null-pointer-is-valid", ["foo", "bar"]]} {
+llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null_pointer_is_valid", ["foo", "bar"]]} {
   llvm.return
 }
 
 // CHECK: attributes #[[ATTR_GROUP]] = {
 // CHECK-DAG: noinline
 // CHECK-DAG: alignstack=4
-// CHECK-DAG: "null-pointer-is-valid"
+// CHECK-DAG: null_pointer_is_valid
 // CHECK-DAG: "foo"="bar"
Index: 

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: jurahul, kuter.

In D78862#2027254 , @arsenm wrote:

> In D78862#2012560 , @jdoerfert wrote:
>
> > I think this is an improvement over the status quo and it looks fine to me.
> >
> > @arsenm I agree that we should tie this to the data layout (or at least 
> > should try) but I guess there are open questions to answer and code to 
> > write.
> >  I propose to accept this and work on the DL patch after. WDYT?
>
>
> Seems ok, but it's still burning an enum value which I guess isn't super 
> important. With the datalayout property, we might really want the inverse 
> attribute


Given that we lifted the limits on enum attributes I don't think this cost is 
high. Since we do not have a scheduled alternative, I think this should land.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D78862#2012560 , @jdoerfert wrote:

> I think this is an improvement over the status quo and it looks fine to me.
>
> @arsenm I agree that we should tie this to the data layout (or at least 
> should try) but I guess there are open questions to answer and code to write.
>  I propose to accept this and work on the DL patch after. WDYT?


Seems ok, but it's still burning an enum value which I guess isn't super 
important. With the datalayout property, we might really want the inverse 
attribute

> 
> 
> In D78862#2008831 , @manojgupta 
> wrote:
> 
>> @nikic Thanks for the work.
>>
>> In D78862#2003684 , @arsenm wrote:
>>
>> > FWIW I think this attribute should be replaced with a data layout 
>> > property, so this would eventually be removed
>>
>>
>> @arsenm  Is there any work planned on moving to data layout? Moving to data 
>> layout may affect cross TU inlining e.g. LTO where 1 TU is compiled with 
>> `-fno-delete-null-pointer-checks` and other TU is not. There might be other 
>> potential impact that we might not know yet.
> 
> 
> If we link modules with mismatching data layouts we can/should deal with this 
> by utilizing more address spaces. That is, change the address space in one 
> module to a fresh one to keep the properties alive. There need to be rules 
> for this and infrastructure but something similar might be needed for 
> heterogeneous IR modules soon. Different story though. We can also combine 
> the attribute and the data layout if necessary, though I'm not a fan.

This sounds really problematic and would require way more knowledge of target 
address spaces. I don't think this will work


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.
Herald added a subscriber: stephenneuendorffer.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Nikita Popov via Phabricator via cfe-commits]

nikic updated this revision to Diff 261375.
nikic added a comment.

Fix rebase mistake in test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/delete-null-pointer-checks.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/include/llvm/IR/AutoUpgrade.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Analysis/MemorySSA/cyclicphi.ll
  llvm/test/Analysis/ValueTracking/assume.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/Attributor/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Attributor/align.ll
  llvm/test/Transforms/Attributor/nocapture-1.ll
  llvm/test/Transforms/Attributor/nonnull.ll
  llvm/test/Transforms/Attributor/norecurse.ll
  llvm/test/Transforms/Attributor/undefined_behavior.ll
  llvm/test/Transforms/CorrelatedValuePropagation/non-null.ll
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/MallocSROA-section-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi.ll
  llvm/test/Transforms/GlobalOpt/load-store-global-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2.ll
  llvm/test/Transforms/GlobalOpt/storepointer-compare-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/storepointer-no-null-opt.ll
  llvm/test/Transforms/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Inline/attributes.ll
  llvm/test/Transforms/InstCombine/atomic.ll
  llvm/test/Transforms/InstCombine/invariant.group.ll
  llvm/test/Transforms/InstCombine/invoke.ll
  llvm/test/Transforms/InstCombine/lifetime-no-null-opt.ll
  llvm/test/Transforms/InstCombine/load.ll
  llvm/test/Transforms/InstCombine/mem-deref-bytes.ll
  llvm/test/Transforms/InstCombine/memchr.ll
  llvm/test/Transforms/InstCombine/memcpy-addrspace.ll
  llvm/test/Transforms/InstCombine/memcpy-from-global.ll
  llvm/test/Transforms/InstCombine/memrchr.ll
  llvm/test/Transforms/InstCombine/select.ll
  llvm/test/Transforms/InstCombine/store.ll
  llvm/test/Transforms/InstCombine/strchr-1.ll
  llvm/test/Transforms/InstCombine/strcpy_chk-64.ll
  llvm/test/Transforms/InstCombine/strlen-1.ll
  llvm/test/Transforms/InstCombine/strncat-2.ll
  llvm/test/Transforms/InstCombine/strncmp-1.ll
  llvm/test/Transforms/InstCombine/strrchr-1.ll
  llvm/test/Transforms/InstCombine/strstr-1.ll
  llvm/test/Transforms/InstCombine/wcslen-1.ll
  llvm/test/Transforms/InstSimplify/compare.ll
  llvm/test/Transforms/LoopIdiom/pr28196.ll
  llvm/test/Transforms/LoopVersioning/lcssa.ll
  llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
  llvm/test/Transforms/SimplifyCFG/invoke.ll
  llvm/test/Transforms/SimplifyCFG/phi-undef-loadstore.ll
  llvm/test/Transforms/SimplifyCFG/trap-no-null-opt-debugloc.ll
  llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
  llvm/test/Transforms/Util/assume-builder.ll
  mlir/test/Target/llvmir.mlir

Index: mlir/test/Target/llvmir.mlir
===
--- mlir/test/Target/llvmir.mlir
+++ mlir/test/Target/llvmir.mlir
@@ -1205,12 +1205,12 @@
 
 // CHECK-LABEL: @passthrough
 // CHECK: #[[ATTR_GROUP:[0-9]*]]
-llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null-pointer-is-valid", ["foo", "bar"]]} {
+llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null_pointer_is_valid", ["foo", "bar"]]} {
   llvm.return
 }
 
 // CHECK: attributes #[[ATTR_GROUP]] = {
 // CHECK-DAG: noinline
 // CHECK-DAG: alignstack=4
-// CHECK-DAG: "null-pointer-is-valid"
+// CHECK-DAG: null_pointer_is_valid
 // CHECK-DAG: "foo"="bar"
Index: llvm/test/Transforms/Util/assume-builder.ll
===
--- llvm/test/Transforms/Util/assume-builder.ll
+++ 

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Nikita Popov via Phabricator via cfe-commits]

nikic updated this revision to Diff 261279.
nikic marked an inline comment as done.
nikic added a comment.

Rebase over committed NFC changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/delete-null-pointer-checks.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/include/llvm/IR/AutoUpgrade.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Analysis/MemorySSA/cyclicphi.ll
  llvm/test/Analysis/ValueTracking/assume.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/Attributor/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Attributor/align.ll
  llvm/test/Transforms/Attributor/nocapture-1.ll
  llvm/test/Transforms/Attributor/nonnull.ll
  llvm/test/Transforms/Attributor/norecurse.ll
  llvm/test/Transforms/Attributor/undefined_behavior.ll
  llvm/test/Transforms/CorrelatedValuePropagation/non-null.ll
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/MallocSROA-section-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi.ll
  llvm/test/Transforms/GlobalOpt/load-store-global-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2.ll
  llvm/test/Transforms/GlobalOpt/storepointer-compare-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/storepointer-no-null-opt.ll
  llvm/test/Transforms/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Inline/attributes.ll
  llvm/test/Transforms/InstCombine/atomic.ll
  llvm/test/Transforms/InstCombine/invariant.group.ll
  llvm/test/Transforms/InstCombine/invoke.ll
  llvm/test/Transforms/InstCombine/lifetime-no-null-opt.ll
  llvm/test/Transforms/InstCombine/load.ll
  llvm/test/Transforms/InstCombine/mem-deref-bytes.ll
  llvm/test/Transforms/InstCombine/memchr.ll
  llvm/test/Transforms/InstCombine/memcpy-addrspace.ll
  llvm/test/Transforms/InstCombine/memcpy-from-global.ll
  llvm/test/Transforms/InstCombine/memrchr.ll
  llvm/test/Transforms/InstCombine/select.ll
  llvm/test/Transforms/InstCombine/store.ll
  llvm/test/Transforms/InstCombine/strchr-1.ll
  llvm/test/Transforms/InstCombine/strcpy_chk-64.ll
  llvm/test/Transforms/InstCombine/strlen-1.ll
  llvm/test/Transforms/InstCombine/strncat-2.ll
  llvm/test/Transforms/InstCombine/strncmp-1.ll
  llvm/test/Transforms/InstCombine/strrchr-1.ll
  llvm/test/Transforms/InstCombine/strstr-1.ll
  llvm/test/Transforms/InstCombine/wcslen-1.ll
  llvm/test/Transforms/InstSimplify/compare.ll
  llvm/test/Transforms/LoopIdiom/pr28196.ll
  llvm/test/Transforms/LoopVersioning/lcssa.ll
  llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
  llvm/test/Transforms/SimplifyCFG/invoke.ll
  llvm/test/Transforms/SimplifyCFG/phi-undef-loadstore.ll
  llvm/test/Transforms/SimplifyCFG/trap-no-null-opt-debugloc.ll
  llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
  llvm/test/Transforms/Util/assume-builder.ll
  mlir/test/Target/llvmir.mlir

Index: mlir/test/Target/llvmir.mlir
===
--- mlir/test/Target/llvmir.mlir
+++ mlir/test/Target/llvmir.mlir
@@ -1205,12 +1205,12 @@
 
 // CHECK-LABEL: @passthrough
 // CHECK: #[[ATTR_GROUP:[0-9]*]]
-llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null-pointer-is-valid", ["foo", "bar"]]} {
+llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null_pointer_is_valid", ["foo", "bar"]]} {
   llvm.return
 }
 
 // CHECK: attributes #[[ATTR_GROUP]] = {
 // CHECK-DAG: noinline
 // CHECK-DAG: alignstack=4
-// CHECK-DAG: "null-pointer-is-valid"
+// CHECK-DAG: null_pointer_is_valid
 // CHECK-DAG: "foo"="bar"
Index: llvm/test/Transforms/Util/assume-builder.ll
===
--- 

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I think this is an improvement over the status quo and it looks fine to me.

@arsenm I agree that we should tie this to the data layout (or at least should 
try) but I guess there are open questions to answer and code to write.
I propose to accept this and work on the DL patch after. WDYT?

In D78862#2008831 , @manojgupta wrote:

> @nikic Thanks for the work.
>
> In D78862#2003684 , @arsenm wrote:
>
> > FWIW I think this attribute should be replaced with a data layout property, 
> > so this would eventually be removed
>
>
> @arsenm  Is there any work planned on moving to data layout? Moving to data 
> layout may affect cross TU inlining e.g. LTO where 1 TU is compiled with 
> `-fno-delete-null-pointer-checks` and other TU is not. There might be other 
> potential impact that we might not know yet.


If we link modules with mismatching data layouts we can/should deal with this 
by utilizing more address spaces. That is, change the address space in one 
module to a fresh one to keep the properties alive. There need to be rules for 
this and infrastructure but something similar might be needed for heterogeneous 
IR modules soon. Different story though. We can also combine the attribute and 
the data layout if necessary, though I'm not a fan.




Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1321
-I == Attribute::NoSync)
-  continue;
 if (uint64_t A = (Val & getRawAttributeMask(I))) {

I guess this change can go in as NFC simplification right away.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

@nikic Thanks for the work.

In D78862#2003684 , @arsenm wrote:

> FWIW I think this attribute should be replaced with a data layout property, 
> so this would eventually be removed


@arsenm  Is there any work planned on moving to data layout? Moving to data 
layout may affect cross TU inlining e.g. LTO where 1 TU is compiled with 
`-fno-delete-null-pointer-checks` and other TU is not. There might be other 
potential impact that we might not know yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

@arsenm If the `null-pointer-is-valid` attribute is moved into the data layout, 
I'm wondering how Clang's `-fno-delete-null-pointer-checks` option would work 
or what it would be replaced with. In Rust it is possible to define a custom 
target, which also defines a custom data-layout, though I think that also needs 
to be compatible with the base target. I couldn't find any information on how 
to use a custom data layout in Clang. As such, I suspect that to preserve 
existing functionality we'd need support //both// for specifying this 
per-addrspace in the DL and as a function attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

In D78862#2003684 , @arsenm wrote:

> FWIW I think this attribute should be replaced with a data layout property, 
> so this would eventually be removed


Is this change planned more for the near term or the long term? I'm not really 
familiar with ongoing addrspace related work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D78862#2003684 , @arsenm wrote:

> FWIW I think this attribute should be replaced with a data layout property, 
> so this would eventually be removed


Also would be address space specific


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

FWIW I think this attribute should be replaced with a data layout property, so 
this would eventually be removed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

[Nikita Popov via Phabricator via cfe-commits]

nikic updated this revision to Diff 260105.
nikic added a comment.
Herald added subscribers: cfe-commits, Kayjukh, frgossen, grosul1, Joonsoo, 
liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, 
shauheen, jpienaar, rriddle, mehdi_amini, asbirlea, jfb, george.burgess.iv.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: ftynse.
Herald added a project: clang.

Update tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78862/new/

https://reviews.llvm.org/D78862

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/delete-null-pointer-checks.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/include/llvm/IR/AutoUpgrade.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Analysis/MemorySSA/cyclicphi.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/Attributor/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Attributor/align.ll
  llvm/test/Transforms/Attributor/nocapture-1.ll
  llvm/test/Transforms/Attributor/nonnull.ll
  llvm/test/Transforms/Attributor/norecurse.ll
  llvm/test/Transforms/Attributor/undefined_behavior.ll
  llvm/test/Transforms/CorrelatedValuePropagation/non-null.ll
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/MallocSROA-section-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-1.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-2.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-3.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-4.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/heap-sra-phi.ll
  llvm/test/Transforms/GlobalOpt/load-store-global-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-1.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/malloc-promote-2.ll
  llvm/test/Transforms/GlobalOpt/storepointer-compare-no-null-opt.ll
  llvm/test/Transforms/GlobalOpt/storepointer-no-null-opt.ll
  llvm/test/Transforms/IPConstantProp/PR26044.ll
  llvm/test/Transforms/Inline/attributes.ll
  llvm/test/Transforms/InstCombine/atomic.ll
  llvm/test/Transforms/InstCombine/invariant.group.ll
  llvm/test/Transforms/InstCombine/invoke.ll
  llvm/test/Transforms/InstCombine/lifetime-no-null-opt.ll
  llvm/test/Transforms/InstCombine/load.ll
  llvm/test/Transforms/InstCombine/mem-deref-bytes.ll
  llvm/test/Transforms/InstCombine/memchr.ll
  llvm/test/Transforms/InstCombine/memcpy-addrspace.ll
  llvm/test/Transforms/InstCombine/memcpy-from-global.ll
  llvm/test/Transforms/InstCombine/memrchr.ll
  llvm/test/Transforms/InstCombine/select.ll
  llvm/test/Transforms/InstCombine/store.ll
  llvm/test/Transforms/InstCombine/strchr-1.ll
  llvm/test/Transforms/InstCombine/strcpy_chk-64.ll
  llvm/test/Transforms/InstCombine/strlen-1.ll
  llvm/test/Transforms/InstCombine/strncat-2.ll
  llvm/test/Transforms/InstCombine/strncmp-1.ll
  llvm/test/Transforms/InstCombine/strrchr-1.ll
  llvm/test/Transforms/InstCombine/strstr-1.ll
  llvm/test/Transforms/InstCombine/wcslen-1.ll
  llvm/test/Transforms/InstSimplify/compare.ll
  llvm/test/Transforms/LoopIdiom/pr28196.ll
  llvm/test/Transforms/LoopVersioning/lcssa.ll
  llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
  llvm/test/Transforms/SimplifyCFG/invoke.ll
  llvm/test/Transforms/SimplifyCFG/phi-undef-loadstore.ll
  llvm/test/Transforms/SimplifyCFG/trap-no-null-opt-debugloc.ll
  llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
  llvm/test/Transforms/Util/assume-builder.ll
  mlir/test/Target/llvmir.mlir

Index: mlir/test/Target/llvmir.mlir
===
--- mlir/test/Target/llvmir.mlir
+++ mlir/test/Target/llvmir.mlir
@@ -1205,12 +1205,12 @@
 
 // CHECK-LABEL: @passthrough
 // CHECK: #[[ATTR_GROUP:[0-9]*]]
-llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null-pointer-is-valid", ["foo", "bar"]]} {
+llvm.func @passthrough() attributes {passthrough = ["noinline", ["alignstack", "4"], "null_pointer_is_valid", ["foo", "bar"]]} {
   llvm.return
 }
 
 // CHECK: attributes #[[ATTR_GROUP]] = {
 // CHECK-DAG: noinline
 // CHECK-DAG: