[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser accepted this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

LGTM

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139720

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

[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I'm fine with handling the return for common as a separate change, if necessary.
Is the error produced now because it passes back the incorrect option rather 
than quietly changing it to something appropriate as it did before?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139720

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

[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a subscriber: hubert.reinterpretcast.
jamieschmeiser added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:54
+auto TargetCPUName = llvm::StringSwitch(CPUName)
+ .Case("common", "generic")
+ .Case("440fp", "440")

qiongsiwu1 wrote:
> jamieschmeiser wrote:
> > This seems strange.  If the option is "generic", it calls 
> > getPPCGenericTargetCPU(), but if it is "common", it returns "generic."  I 
> > think you may want to also call getPPCGenericTargetCPU() here.  There 
> > should probably also be an assume where this returns that it didn't return 
> > "generic" if that is the intended result.  Also, there should also be tests 
> > for what happens when "generic" and "common" are specified.
> I agree this `generic` vs `common` behaviour is strange. They way this patch 
> handles `generic` vs `common` keeps the existing behaviour. In short, before 
> this patch, `clang` already treats `-mcpu=generic` and `-mcpu=common` 
> differently. 
> 
> When `-mcpu=generic` is specified, we [[ 
> https://github.com/llvm/llvm-project/blob/d3c3de63ce8416ab2dee7f784e54b00a2aa8ed85/clang/lib/Driver/ToolChains/CommonArgs.cpp#L420
>  | do some processing depending ]] on the OS and architecture (effectively 
> calling `getPPCGenericTargetCPU`). This happens because `generic` is not one 
> of the cases of the big `StringSwitch`, and we return an empty string from 
> `ppc::getPPCTargetCPU`.  
> 
> On the other hand, when `-mcpu=common` is specified, the `StringSwitch` maps 
> `common` to `generic`, and we simply returns `generic` as the target CPU name 
> [[ 
> https://github.com/llvm/llvm-project/blob/d3c3de63ce8416ab2dee7f784e54b00a2aa8ed85/clang/lib/Driver/ToolChains/CommonArgs.cpp#L418
>  | here ]]. 
> 
> **//Is this behaviour what we expect? //**If it is not, I will add a bug fix 
> (with this patch or with a different patch if a separate one is easier to 
> review). We only have an [[ 
> https://github.com/llvm/llvm-project/blob/2c69e1d19a2b8bcf27ef1c5a4b5cc0410ed81a52/clang/test/Driver/ppc-cpus.c#L23
>  |  existing test case ]] for `generic`, but not for `common`. I will add one 
> with the bug fix. 
Yeah, I saw that you weren't changing the behaviour; I just suspect that it was 
an existing bug...Not sure who to check with about that @nemanjai? 
@hubert.reinterpretcast?



Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:77
+ .Default(CPUName);
+return TargetCPUName.str();
   }

qiongsiwu1 wrote:
> jamieschmeiser wrote:
> > Why did you change the type from const char *?  Couldn't you use 
> > CPUName->data() in the default instead?  With your change, I think it may 
> > need to create StringRefs around all of the choices and then just get the 
> > string from them.
> I was worried about the [[ 
> https://github.com/llvm/llvm-project/blob/d3c3de63ce8416ab2dee7f784e54b00a2aa8ed85/llvm/include/llvm/ADT/StringRef.h#L129
>  | comment ]] on top of `StringRef::data()` which says "Get a pointer to the 
> start of the string (which may not be null terminated).". I was not sure what 
> would happen if the data inside `CPUName` was not null terminated (it might 
> be fine) so I thought it would be safer to use the `StringRef`s directly 
> instead. 
Probably the same sort of thing that would happen with creating a StringRef 
since that calls strlen to set the size of the string it is wrapping :-)


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

https://reviews.llvm.org/D139720

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

[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser requested changes to this revision.
jamieschmeiser added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:54
+auto TargetCPUName = llvm::StringSwitch(CPUName)
+ .Case("common", "generic")
+ .Case("440fp", "440")

This seems strange.  If the option is "generic", it calls 
getPPCGenericTargetCPU(), but if it is "common", it returns "generic."  I think 
you may want to also call getPPCGenericTargetCPU() here.  There should probably 
also be an assume where this returns that it didn't return "generic" if that is 
the intended result.  Also, there should also be tests for what happens when 
"generic" and "common" are specified.



Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:77
+ .Default(CPUName);
+return TargetCPUName.str();
   }

Why did you change the type from const char *?  Couldn't you use 
CPUName->data() in the default instead?  With your change, I think it may need 
to create StringRefs around all of the choices and then just get the string 
from them.


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

https://reviews.llvm.org/D139720

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

[PATCH] D135658: demangle OptFunction trace names

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser accepted this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135658

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

[PATCH] D135658: demangle OptFunction trace names

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

Is the mode change on check-time-trace-sections.py intended?  I did a count of 
python files under llvm-project and < 10% are 755.  I'm not saying it is better 
one way or the other; I'm just ensuring it is intentional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135658

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

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

Yes, I agree it is incomplete (aside from being incorrect here :-)  I've just 
been asking to ensure that my understanding of freeze is correct.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136284

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

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

In D136284#3874614 , @nlopes wrote:

> In D136284#3874596 , 
> @jamieschmeiser wrote:
>
>> In D136284#3874492 , @nikic wrote:
>>
>>> At least in C++, working with uninitialized memory is pretty much always 
>>> immediate undefined behavior, see https://eel.is/c++draft/basic.indet for 
>>> the relevant wording. The only exception are "copy-like" operations on 
>>> unsigned character types, which comparisons do not fall under.
>>>
>>> I believe the C specification is less clear cut about this, but Clang and 
>>> LLVM assume basically the same to also hold for C code.
>>
>> What version of the C++ standard is this?  Every version that I have seen 
>> has basics as section 3 and I cannot find this section, nor anything 
>> similar.  Section 6 is Statements.
>
> That discussion is orthogonal to this patch.
> This patch is not desired because it's not needed per the current LLVM IR 
> semantics. If you want to change something, you need to start by proposing a 
> change to the LLVM IR semantics. You'll need to justify why it's needed, why 
> it's correct, the perf impact, how to make it backwards compatible, why it's 
> better than the proposals over the table right now.
>
> Anyway, a patch like this solves no problem. LLVM allows loads to be 
> duplicated. Your patch does nothing to prevent that and to ensure all loads 
> see the same value. The issue is way more complicated than what this patch 
> implies.

I'm not trying to flog a dead horse (I've already abandoned this) but I am 
trying to understand this statement.  I do not dispute that there may be other 
situations similar to this but, assuming that we did want to ensure that the 
loads had the same value, why is freezing them at this point not the correct 
thing to do?  Whether they are poison or undef, freezing them would ensure that 
they compare equal.  Yes, I understand it may have performance impacts, there 
may be better ways, etc.  But, ignoring all that, isn't this exactly what 
freeze is designed for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136284

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

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser abandoned this revision.
jamieschmeiser added a comment.

I checked with a member of the C++ standards committee and he verified that 
comparing an uninitialized value against itself is, indeed, undefined 
behaviour, in the general case.  I am abandoning this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136284

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

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

In D136284#3874492 , @nikic wrote:

> At least in C++, working with uninitialized memory is pretty much always 
> immediate undefined behavior, see https://eel.is/c++draft/basic.indet for the 
> relevant wording. The only exception are "copy-like" operations on unsigned 
> character types, which comparisons do not fall under.
>
> I believe the C specification is less clear cut about this, but Clang and 
> LLVM assume basically the same to also hold for C code.

What version of the C++ standard is this?  Every version that I have seen has 
basics as section 3 and I cannot find this section, nor anything similar.  
Section 6 is Statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136284

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

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

In D136284#3869216 , @nikic wrote:

> The current behavior here is intentional -- in fact, LLVM will move towards 
> returning poison for loads from uninitialized memory in the future (though 
> precisely how this will happen is still uncertain, see 
> https://discourse.llvm.org/t/rfc-making-bit-field-codegen-poison-compatible/63250
>  for some recent discussion on the topic).

The current behaviour implies that the content of uninitialized memory is 
volatile, which is not correct.  One would still need the freeze to ensure that 
2 loads of the same uninitialized memory are the same, whether this is undef or 
poison.  So this would not affect that future change and would still be 
required.  Besides, are you sure that poison is appropriate here?  Loading 
uninitialized memory is not erroneous; it is undefined.  Comparing the same 
uninitialized memory, however, is defined, hence the freeze.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136284

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

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

In D136284#3869306 , @nlopes wrote:

>> However, multiple loads of the same memory must be equal
>
> This premise is incorrect w.r.t. with current semantics, which say that loads 
> return undef for uninit memory.
>
> As Nikita mentioned we are working towards changing this semantics. The work 
> is ongoing, but we pivoted to make a couple of improvements in other areas to 
> avoid perf regressions. For example, see the clang !noundef patches.

This is a C/C++ language semantics statement.  Yes, I realize that LLVM is not 
language specific, but this is what is generated for this, and also, what 
freeze appears specifically designed to handle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136284

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

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added reviewers: nikic, tstellar.
Herald added subscribers: nlopes, kosarev, kerbowa, hiraditya, arichardson, 
jvesely.
Herald added a project: All.
jamieschmeiser requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

SROA will replace loads of an alloca with undef when there are no
prior stores.  However, multiple loads of the same memory must be
equal.  Insert freeze instructions so that loads of the same alloca
with no prior store will compare correctly.

See new lit test /Transforms/SROA/same-promoted-undefs.ll for sample
IR that is fixed by this change.

Also fix up existing lit tests.  @tstellar, please examine the AMD test changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136284

Files:
  clang/test/CodeGen/LoongArch/inline-asm-gcc-regs.c
  clang/test/CodeGenCXX/return.cpp
  clang/test/CodeGenOpenCL/overload.cl
  llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
  llvm/test/CodeGen/AMDGPU/promote-alloca-vector-to-vector.ll
  llvm/test/CodeGen/AMDGPU/vector-alloca-limits.ll
  llvm/test/Transforms/Mem2Reg/pr24179.ll
  llvm/test/Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll
  llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
  llvm/test/Transforms/SROA/address-spaces.ll
  llvm/test/Transforms/SROA/addrspacecast.ll
  llvm/test/Transforms/SROA/alloca-address-space.ll
  llvm/test/Transforms/SROA/basictest.ll
  llvm/test/Transforms/SROA/phi-and-select.ll
  llvm/test/Transforms/SROA/phi-gep.ll
  llvm/test/Transforms/SROA/phi-with-duplicate-pred.ll
  llvm/test/Transforms/SROA/pr37267.ll
  llvm/test/Transforms/SROA/same-promoted-undefs.ll
  llvm/test/Transforms/SROA/scalable-vectors.ll
  llvm/test/Transforms/SROA/select-load.ll
  llvm/test/Transforms/SROA/slice-width.ll
  llvm/test/Transforms/SROA/sroa-common-type-fail-promotion.ll
  llvm/test/Transforms/SROA/vector-conversion.ll
  llvm/test/Transforms/SROA/vector-promotion.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/amdgpu_asm.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/amdgpu_isel.ll.expected

Index: llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/amdgpu_isel.ll.expected
===
--- llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/amdgpu_isel.ll.expected
+++ llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/amdgpu_isel.ll.expected
@@ -3,12 +3,18 @@
 
 define i64 @i64_test(i64 %i) nounwind readnone {
 ; CHECK-LABEL: i64_test:
-; CHECK:   SelectionDAG has 9 nodes:
+; CHECK:   SelectionDAG has 19 nodes:
 ; CHECK-NEXT:t0: ch,glue = EntryToken
-; CHECK-NEXT:t11: ch,glue = CopyToReg t0, Register:i32 $vgpr0, IMPLICIT_DEF:i32
-; CHECK-NEXT:t17: i32 = V_MOV_B32_e32 TargetConstant:i32<0>
-; CHECK-NEXT:t13: ch,glue = CopyToReg t11, Register:i32 $vgpr1, t17, t11:1
-; CHECK-NEXT:t14: ch = SI_RETURN Register:i32 $vgpr0, Register:i32 $vgpr1, t13, t13:1
+; CHECK-NEXT:t2: i32,ch = CopyFromReg # D:1 t0, Register:i32 %0
+; CHECK-NEXT:t4: i32,ch = CopyFromReg # D:1 t0, Register:i32 %1
+; CHECK-NEXT:t31: i64 = REG_SEQUENCE # D:1 TargetConstant:i32<55>, t2, TargetConstant:i32<3>, t4, TargetConstant:i32<11>
+; CHECK-NEXT:t7: i64 = COPY IMPLICIT_DEF:i64
+; CHECK-NEXT:t8: i64 = V_ADD_U64_PSEUDO # D:1 t31, t7
+; CHECK-NEXT:t21: i32 = EXTRACT_SUBREG # D:1 t8, TargetConstant:i32<3>
+; CHECK-NEXT:t14: ch,glue = CopyToReg # D:1 t0, Register:i32 $vgpr0, t21
+; CHECK-NEXT:t25: i32 = EXTRACT_SUBREG # D:1 t8, TargetConstant:i32<11>
+; CHECK-NEXT:t16: ch,glue = CopyToReg # D:1 t14, Register:i32 $vgpr1, t25, t14:1
+; CHECK-NEXT:t17: ch = SI_RETURN # D:1 Register:i32 $vgpr0, Register:i32 $vgpr1, t16, t16:1
 ; CHECK-EMPTY:
   %loc = alloca i64
   %j = load i64, i64 * %loc
@@ -18,12 +24,15 @@
 
 define i64 @i32_test(i32 %i) nounwind readnone {
 ; CHECK-LABEL: i32_test:
-; CHECK:   SelectionDAG has 8 nodes:
-; CHECK-NEXT:t5: i32 = V_MOV_B32_e32 TargetConstant:i32<0>
+; CHECK:   SelectionDAG has 14 nodes:
 ; CHECK-NEXT:t0: ch,glue = EntryToken
-; CHECK-NEXT:t7: ch,glue = CopyToReg t0, Register:i32 $vgpr0, t5
-; CHECK-NEXT:t9: ch,glue = CopyToReg t7, Register:i32 $vgpr1, t5, t7:1
-; CHECK-NEXT:t10: ch = SI_RETURN Register:i32 $vgpr0, Register:i32 $vgpr1, t9, t9:1
+; CHECK-NEXT:t2: i32,ch = CopyFromReg # D:1 t0, Register:i32 %0
+; CHECK-NEXT:t4: i32 = COPY IMPLICIT_DEF:i32
+; CHECK-NEXT:t5: i32,i1 = V_ADD_CO_U32_e64 # D:1 t2, t4, TargetConstant:i1<0>
+; CHECK-NEXT:t12: ch,glue = CopyToReg # D:1 t0, Register:i32 $vgpr0, t5
+; CHECK-NEXT:t20: i32 = V_MOV_B32_e32 TargetConstant:i32<0>
+; CHECK-NEXT:t14: ch,glue = CopyToReg # D:1 t12, Register:i32 $vgpr1, t20, t12:1
+; CHECK-NEXT:t15: ch = SI_RETURN # D:1 Register:i32 $vgpr0, Register:i32 $vgpr1, t14, t14:1
 ; CHECK-EMPTY:
   %lo

[PATCH] D133662: [Clang] Change -ftime-trace storing path and support multiple compilation jobs

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

Great.  Also, something you may want to consider, either as part of or after 
you land this code.  This really is a specific instance of a more generic 
problem: setting up option handling for something that saves results in a file 
for each compilation.  This is equally useful for other things such as listings 
and could also be used by something like print-changed (which currently just 
outputs to the stream), opt stats reporting, etc.  This code could be organized 
as a function (or possibly an object, depends...) that takes  a string for the 
extension, a lambda/template for the virtual call on whether to add the option 
to a tool so that off-handling, platform-isms, and where files are saved would 
all be captured neatly and would be re-usable.  InferTimeTrace and getPath, 
off-loading, platform-isms would be captured in a generic call that would look 
something like (in this instance)

  PerFileTraceGenerator(".json",
   [](Tool &T, Args &Args)->bool{ return T->supportsTimeTrace() && 
Args.hasArg(options::OPT_ftime_trace, options::OPT_ftime_trace_EQ; },
   "-ftime-trace="); 

Each option that needs per file output would just call this function 
appropriately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133662

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

[PATCH] D133662: [Clang] Change -ftime-trace storing path and support multiple compilation jobs

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I had an email exchange with @dongjunduo about this situation.  He was a GSoC 
student that @Whitney and I were mentoring for the past summer.  He agrees that 
your approach is cleaner.  There appears to be two parts to your work.  First, 
you implemented the determining and passing of the options differently, and 
secondly, you improved the handling of off-loading and system specific file 
handling.  Based on your earlier response, we proposed to him the following and 
he agrees that it seems appropriate.  Could you please add comments to 
https://reviews.llvm.org/D131469 and he will work with you to change his code 
to reflect searching for -o and using the virtual functions.  Then, if @MaskRay 
agrees, he can land his code and finish up his GSoC work.  You can then add 
your extensions of off-loading and file-handling.  Is this acceptable to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133662

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

[PATCH] D133662: [Clang] WIP: Change -ftime-trace storing path and support multiple compilation jobs

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I'm a little confused as to what is being proposed here.  Is this building on 
D131469  or is it an alternative?  It seems 
that there are portions of the code from D131469 
 included in these changes, which implies 
that you are building on it.  I think this patch is premature in that the other 
patch has not yet landed (@MaskRay has asked for revisions that @dongjunduo has 
made but is waiting for review).  When that patch has landed, this could be 
reposted based on those changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133662

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

[PATCH] D133587: Loop names used in reporting can grow very large

[Jamie Schmeiser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e3ac7969039: Loop names used in reporting can grow very 
large (authored by jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133587

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pass-ordering.ll
  llvm/test/Other/loop-pass-printer.ll
  llvm/test/Other/loopnest-pass-ordering.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/pass-pipeline-parsing.ll
  llvm/test/Other/print-before-after.ll
  llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
  llvm/test/Transforms/LoopPredication/preserve-bpi.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/LoopUnroll/revisit.ll
  llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
  
llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
@@ -4,7 +4,7 @@
 ; Running loop-distribute will result in LoopAccessAnalysis being required and
 ; cached in the LoopAnalysisManagerFunctionProxy.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner
 
 
 ; Then simple-loop-unswitch is removing/replacing some loops (resulting in
@@ -17,7 +17,7 @@
 ; SimpleLoopUnswitch not marking the Loop as removed, so we missed clearing
 ; the analysis caches.
 ;
-; CHECK: Running pass: SimpleLoopUnswitchPass on Loop at depth 1 containing: %loop_begin,%loop_b,%loop_b_inner,%loop_b_inner_exit,%loop_a,%loop_a_inner,%loop_a_inner_exit,%latch
+; CHECK: Running pass: SimpleLoopUnswitchPass on loop_begin
 ; CHECK-NEXT: Running analysis: OuterAnalysisManagerProxy
 ; CHECK-NEXT: Clearing all analysis results for: loop_a_inner
 
@@ -27,7 +27,7 @@
 ; loop_a_inner.us). This kind of verifies that it was correct to remove the
 ; loop_a_inner related analysis above.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner.us
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner.us
 
 
 define i32 @test6(i1* %ptr, i1 %cond1, i32* %a.ptr, i32* %b.ptr) {
Index: llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
===
--- llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
+++ llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
@@ -8,22 +8,22 @@
 ; CHECK: Running analysis: LoopAnalysis
 ; CHECK: Running analysis: InnerAnalysisManagerProxy<
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner2.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner2.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %outer.header
+; CHECK: Running analysis: LoopAccessAnalysis on outer.header
 ; CHECK: Running pass: LoopUnrollPass
 ; CHECK: Clearing all analysis results for: inner2.header
 ; CHECK: Clearing all analysis results for: outer.header
 ; CHECK: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header
 ; CHECK-NOT: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header.1
 ; CHECK: Running pass: LoopAccessInfoPrinterPass
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Loop access info in function 'test':
 ; CHECK:   inner1.header:
 ; CHECK: Running pass: LoopAccessInfoPrinterPass
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %inner1.header.1
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header.1
 ; CHECK: Loop access info in function 'test':
 ; CHECK:   inner1.header.1:
 
Index: llvm/test/Transforms/LoopUnroll/revisit.ll
===
--- llvm/test/Transforms/LoopUnroll/revisit.ll
+++ llvm/test/Transforms/LoopUnroll/revisit.ll
@@ -39,7 +39,7 @@
 l0.0.0:
   %cond.0.0.0 = load volatile i1, i1* %ptr
   br i1 %cond.0.0.0, label %l0.0.0, label %l0.0.1.ph
-; CHECK: LoopFullUnrollP

[PATCH] D133587: Loop names used in reporting can grow very large

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added reviewers: Whitney, dongjunduo, aeubanks, MaskRay.
Herald added subscribers: ormris, StephenFan, steven_wu, zzheng, hiraditya.
Herald added a project: All.
jamieschmeiser requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

In his work for https://reviews.llvm.org/D131960, @dongjunduo found that
the names for loops generated by getIRName could grow to be extremely
large because they are based on a serialization of the loop.  Replace this with
the name of the loop (which is based on the header) and patch up the
affected lit tests.

It was suggested (in separate conversations) that an analogous
fix be made in  https://reviews.llvm.org/D131960 with a FIXME
comment to resolve the 2 pieces of code so https://reviews.llvm.org/D131960
can be delivered as it is part of a GSoC project that is finishing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133587

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/loop-pass-ordering.ll
  llvm/test/Other/loop-pass-printer.ll
  llvm/test/Other/loopnest-pass-ordering.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/pass-pipeline-parsing.ll
  llvm/test/Other/print-before-after.ll
  llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
  llvm/test/Transforms/LoopPredication/preserve-bpi.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/LoopUnroll/revisit.ll
  llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
  
llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll
@@ -4,7 +4,7 @@
 ; Running loop-distribute will result in LoopAccessAnalysis being required and
 ; cached in the LoopAnalysisManagerFunctionProxy.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner
 
 
 ; Then simple-loop-unswitch is removing/replacing some loops (resulting in
@@ -17,7 +17,7 @@
 ; SimpleLoopUnswitch not marking the Loop as removed, so we missed clearing
 ; the analysis caches.
 ;
-; CHECK: Running pass: SimpleLoopUnswitchPass on Loop at depth 1 containing: %loop_begin,%loop_b,%loop_b_inner,%loop_b_inner_exit,%loop_a,%loop_a_inner,%loop_a_inner_exit,%latch
+; CHECK: Running pass: SimpleLoopUnswitchPass on loop_begin
 ; CHECK-NEXT: Running analysis: OuterAnalysisManagerProxy
 ; CHECK-NEXT: Clearing all analysis results for: loop_a_inner
 
@@ -27,7 +27,7 @@
 ; loop_a_inner.us). This kind of verifies that it was correct to remove the
 ; loop_a_inner related analysis above.
 ;
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %loop_a_inner.us
+; CHECK: Running analysis: LoopAccessAnalysis on loop_a_inner.us
 
 
 define i32 @test6(i1* %ptr, i1 %cond1, i32* %a.ptr, i32* %b.ptr) {
Index: llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
===
--- llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
+++ llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
@@ -8,22 +8,22 @@
 ; CHECK: Running analysis: LoopAnalysis
 ; CHECK: Running analysis: InnerAnalysisManagerProxy<
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner2.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner2.header
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %outer.header
+; CHECK: Running analysis: LoopAccessAnalysis on outer.header
 ; CHECK: Running pass: LoopUnrollPass
 ; CHECK: Clearing all analysis results for: inner2.header
 ; CHECK: Clearing all analysis results for: outer.header
 ; CHECK: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header
 ; CHECK-NOT: Invalidating analysis: LoopAccessAnalysis on {{.*}}inner1.header.1
 ; CHECK: Running pass: LoopAccessInfoPrinterPass
-; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Loop access info in function 'test':
 ; CHECK:   inner1.header:
 ; CHECK: Running pass: LoopAccessIn

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser accepted this revision.
jamieschmeiser added a comment.

If the source is specified with a partial path, I would expect the .json file 
to be in the same directory as the source, so dir1/f.C and dir2/f.C would 
result in dir1/f.json and dir2/f.json.  But this can be a later improvement.  
Let's get this landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4667
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!TimeTrace && !TimeTraceFile) return;
+

The return should be on the next line.  Did you run this through clang-format?  
Is it okay with this on the same line?



Comment at: clang/lib/Driver/Driver.cpp:4674
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {

dongjunduo wrote:
> jamieschmeiser wrote:
> > Can you have a link job without an output filename?  If not, then just have 
> > an assert for !empty.  Again, the negative test and continue might be 
> > easier to understand.
> The output filename should not be empty. 
> 
> If the "-o" is not specified, the output filename may be "a.out".
Yes.  If the output filename should not be empty, then assert that rather than 
testing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

Good progress.




Comment at: clang/lib/Driver/Driver.cpp:97
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 

This include isn't necessary.  There are asserts already in the file so this is 
transitively included.



Comment at: clang/lib/Driver/Driver.cpp:4668
+  // if `-ftime-trace` or `-ftime-trace=` are specified
+  if (TimeTrace || TimeTraceFile) {
+SmallString<128> TracePath("");

I think the quick return style is generally preferred.  In this case, test the 
negative and return.  It is easier to follow and limits indenting.



Comment at: clang/lib/Driver/Driver.cpp:4674
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {

Can you have a link job without an output filename?  If not, then just have an 
assert for !empty.  Again, the negative test and continue might be easier to 
understand.



Comment at: clang/lib/Driver/Driver.cpp:4679
+  TracePath = llvm::sys::path::parent_path(
+  SmallString<128>(J.getOutputFilenames()[0].c_str()));
+} else {

you create the same small string twice.  better to create a temporary.



Comment at: clang/lib/Driver/Driver.cpp:4689
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());

Again, test negative with continue
Do you also want CompileJobClass?



Comment at: clang/lib/Driver/Driver.cpp:4700
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+llvm::sys::path::replace_extension(TracePath, "json");

You are changing TracePath, which will then be altered when you loop back.  I 
think you need to use a copy of TracePath.



Comment at: clang/lib/Driver/Driver.cpp:4714
+}
+
+assert(arg.size() > strlen("-ftime-trace") &&

Can you compute the full path to the directory before the loop and then just 
copy it and add the name in the loop for each file?



Comment at: clang/lib/Driver/Driver.cpp:4725
+auto &JArgs = J.getArguments();
+bool exist = false;
+for (unsigned I = 0; I < JArgs.size(); ++I) {

Variables should start with an uppercase letter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

You should not have debugging information in code that is up for review.  If 
this is debugging information that you plan to leave in for future purposes 
(which I doubt is the case here), you need to protect it so that it isn't 
active unless some option is set.  For example, see LLVM_DEBUG code that lives 
in various opt routines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D128048: Add a new clang option "-ftime-trace="

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser accepted this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

LGTM.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

In D128048#3607295 , @MaskRay wrote:

> You may add `-ftime-trace=` instead of introducing a new spelling.

I like this idea.  There is an example of an optional argument on an option 
with -print-changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added inline comments.



Comment at: clang/test/Driver/check-time-trace-path.cpp:5
+// RUN:   | FileCheck %s
+
+// CHECK:  "beginningOfTime": {{[0-9]{16},}}

This test is the same as the one in check-time-trace.cpp except for the path 
feature.  A test file can have multiple tests in it.  Rather than repeating all 
of the checks and source, it would be better to just put this test in the 
existing file.  Just add lines 1-4 from this file into that file.  If you need 
to special-case parts of it, you can use --check-prefix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

In D128048#3601588 , @Whitney wrote:

> In D128048#3601579 , 
> @jamieschmeiser wrote:
>
>> Can you please use git rebase -i to collapse all the changes into a single 
>> change?  If this isn't done, it is difficult to know what is being reviewed 
>> as the changes only show the differences since your last revision, not all 
>> of the changes.
>
> I can see all the changes, make sure your link is not suffix with "new", or 
> check the history tab to see what is it comparing.

It had the /new on the path.  I removed it and now I can see everything.  
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

Can you please use git rebase -i to collapse all the changes into a single 
change?  If this isn't done, it is difficult to know what is being reviewed as 
the changes only show the differences since your last revision, not all of the 
changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser requested changes to this revision.
jamieschmeiser added a comment.
This revision now requires changes to proceed.

This is a good start.  You should mention in the summary that when -c is not 
specified, the compiler is invoked with -o pointing at /tmp so the .json file 
will currently be placed there, which is confusing.




Comment at: clang/include/clang/Driver/Options.td:2832
+def ftime_trace_path : Joined<["-"], "ftime-trace-path=">, Group,
+  HelpText<"Path which stores the output files of time profiler">,
+  Flags<[CC1Option, CoreOption]>,

which specifies the output files for -ftime-trace



Comment at: clang/tools/driver/cc1_main.cpp:259
+
Path.append(llvm::sys::path::filename(Clang->getFrontendOpts().OutputFile));
+Path.append(".json");
 if (auto profilerOutput = Clang->createOutputFile(

What happens if the path specified does not exist?  This needs to be handled 
(if it isn't) and needs a lit test for this situation also.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D126816: Only issue warning for subtraction involving null pointers on live code paths

[Jamie Schmeiser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGefbf0136b410: Only issue warning for subtraction involving 
null pointers on live code paths (authored by jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126816

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-subtraction.c
  clang/test/Sema/pointer-subtraction.cpp


Index: clang/test/Sema/pointer-subtraction.cpp
===
--- clang/test/Sema/pointer-subtraction.cpp
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // valid in C++
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}} 
expected-warning {{performing pointer subtraction with a null pointer has 
undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10845,8 +10845,10 @@
   if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
 return;
 
-  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
-  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+  S.DiagRuntimeBehavior(Loc, Pointer,
+S.PDiag(diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus
+<< Pointer->getSourceRange());
 }
 
 /// Diagnose invalid arithmetic on two function pointers.


Index: clang/test/Sema/pointer-subtraction.cpp
===
--- clang/test/Sema/pointer-subtraction.cpp
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // valid in C++
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTE

[PATCH] D126816: Only issue warning for subtraction involving null pointers on live code paths

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 433736.
jamieschmeiser edited the summary of this revision.
jamieschmeiser added a comment.

Respond to review comments: add cpp test.


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

https://reviews.llvm.org/D126816

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-subtraction.c
  clang/test/Sema/pointer-subtraction.cpp


Index: clang/test/Sema/pointer-subtraction.cpp
===
--- clang/test/Sema/pointer-subtraction.cpp
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // valid in C++
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}} 
expected-warning {{performing pointer subtraction with a null pointer has 
undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10845,8 +10845,10 @@
   if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
 return;
 
-  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
-  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+  S.DiagRuntimeBehavior(Loc, Pointer,
+S.PDiag(diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus
+<< Pointer->getSourceRange());
 }
 
 /// Diagnose invalid arithmetic on two function pointers.


Index: clang/test/Sema/pointer-subtraction.cpp
===
--- clang/test/Sema/pointer-subtraction.cpp
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // valid in C++
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/lib/Sema/SemaExpr.cpp
=

[PATCH] D126816: Only issue warning for subtraction involving null pointers on live code paths

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added reviewers: anarazel, efriedma.
Herald added a project: All.
jamieschmeiser requested review of this revision.
Herald added a project: clang.

Change the warning produced for subtraction from (or with) a null pointer
to only be produced when the code path is live.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126816

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-subtraction.c


Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}} 
expected-warning {{performing pointer subtraction with a null pointer has 
undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10845,8 +10845,10 @@
   if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
 return;
 
-  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
-  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+  S.DiagRuntimeBehavior(Loc, Pointer,
+S.PDiag(diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus
+<< Pointer->getSourceRange());
 }
 
 /// Diagnose invalid arithmetic on two function pointers.


Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10845,8 +10845,10 @@
   if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
 return;
 
-  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
-  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+  S.DiagRuntimeBehavior(Loc, Pointer,
+S.PDiag(diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus
+<< Pointer->getSourceRange());
 }
 
 /// Diagnose invalid arithmetic on two function pointers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107063: Set TargetCPUName for AIX to default to pwr7.

[Jamie Schmeiser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc3c1826c310c: Set TargetCPUName for AIX to default to pwr7. 
(authored by jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107063

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aix-mcpu-default.c


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -2,17 +2,35 @@
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.2 \
 // RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
-// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
-// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// Check that the target cpu defaults to power7 on AIX7.2 and up.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.2 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
-// CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 when level not specified.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc-ibm-aix \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 when level not specified.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
+// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
 // Check that the user is able to overwrite the default with '-mcpu'.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -403,14 +403,9 @@
 if (!TargetCPUName.empty())
   return TargetCPUName;
 
-if (T.isOSAIX()) {
-  unsigned major, minor, unused_micro;
-  T.getOSVersion(major, minor, unused_micro);
-  // The minimal arch level moved from pwr4 for AIX7.1 to
-  // pwr7 for AIX7.2.
-  TargetCPUName =
-  (major < 7 || (major == 7 && minor < 2)) ? "pwr4" : "pwr7";
-} else if (T.getArch() == llvm::Triple::ppc64le)
+if (T.isOSAIX())
+  TargetCPUName = "pwr7";
+else if (T.getArch() == llvm::Triple::ppc64le)
   TargetCPUName = "ppc64le";
 else if (T.getArch() == llvm::Triple::ppc64)
   TargetCPUName = "ppc64";


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -2,17 +2,35 @@
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.2 \
 // RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
-// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
-// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// Check that the target cpu defaults to power7 on AIX7.2 and up.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.2 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
-// CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Ch

[PATCH] D107063: Set TargetCPUName for AIX to default to pwr7.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 362760.
jamieschmeiser added a comment.

Respond to review comment:  expand testing.


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

https://reviews.llvm.org/D107063

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aix-mcpu-default.c


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -2,17 +2,35 @@
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.2 \
 // RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
-// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
-// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// Check that the target cpu defaults to power7 on AIX7.2 and up.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.2 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
-// CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.1 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 when level not specified.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc-ibm-aix \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 when level not specified.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
+// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
+// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
 // Check that the user is able to overwrite the default with '-mcpu'.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -403,14 +403,9 @@
 if (!TargetCPUName.empty())
   return TargetCPUName;
 
-if (T.isOSAIX()) {
-  unsigned major, minor, unused_micro;
-  T.getOSVersion(major, minor, unused_micro);
-  // The minimal arch level moved from pwr4 for AIX7.1 to
-  // pwr7 for AIX7.2.
-  TargetCPUName =
-  (major < 7 || (major == 7 && minor < 2)) ? "pwr4" : "pwr7";
-} else if (T.getArch() == llvm::Triple::ppc64le)
+if (T.isOSAIX())
+  TargetCPUName = "pwr7";
+else if (T.getArch() == llvm::Triple::ppc64le)
   TargetCPUName = "ppc64le";
 else if (T.getArch() == llvm::Triple::ppc64)
   TargetCPUName = "ppc64";


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -2,17 +2,35 @@
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.2 \
 // RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
-// CHECK-MCPU-DEFAULT-AIX72-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
-// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// Check that the target cpu defaults to power7 on AIX7.2 and up.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.2 \
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
-// CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
-// CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
+// RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX72 %s
+
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
+// RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
+// RUN:-target pow

[PATCH] D107063: Set TargetCPUName for AIX to default to pwr7.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added reviewers: hubert.reinterpretcast, ZarkoCA, stevewan.
jamieschmeiser requested review of this revision.
Herald added a project: clang.

Set the TargetCPUName for AIX to default to pwr7, removing the setting
of it based on the major/minor of the OS version, which previously
set it to pwr4 for AIX 7.1 and earlier.  The old code would also set it to
pwr4 when the OS version was not specified and with the change, it will
default it to pwr7 in all cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107063

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/aix-mcpu-default.c


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -6,13 +6,13 @@
 // CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
 // CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
-// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
 // CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
 // CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
+// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr7"
 
 // Check that the user is able to overwrite the default with '-mcpu'.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -403,14 +403,9 @@
 if (!TargetCPUName.empty())
   return TargetCPUName;
 
-if (T.isOSAIX()) {
-  unsigned major, minor, unused_micro;
-  T.getOSVersion(major, minor, unused_micro);
-  // The minimal arch level moved from pwr4 for AIX7.1 to
-  // pwr7 for AIX7.2.
-  TargetCPUName =
-  (major < 7 || (major == 7 && minor < 2)) ? "pwr4" : "pwr7";
-} else if (T.getArch() == llvm::Triple::ppc64le)
+if (T.isOSAIX())
+  TargetCPUName = "pwr7";
+else if (T.getArch() == llvm::Triple::ppc64le)
   TargetCPUName = "ppc64le";
 else if (T.getArch() == llvm::Triple::ppc64)
   TargetCPUName = "ppc64";


Index: clang/test/Driver/aix-mcpu-default.c
===
--- clang/test/Driver/aix-mcpu-default.c
+++ clang/test/Driver/aix-mcpu-default.c
@@ -6,13 +6,13 @@
 // CHECK-MCPU-DEFAULT-AIX72: {{.*}}clang{{.*}}" "-cc1"
 // CHECK-MCPU-DEFAULT-AIX72: "-target-cpu" "pwr7"
 
-// Check that the target cpu defaults to power4 on AIX7.1 and below.
+// Check that the target cpu defaults to power7 on AIX7.1 and below.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
 // RUN:-target powerpc-ibm-aix7.1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MCPU-DEFAULT-AIX71 %s
 // CHECK-MCPU-DEFAULT-AIX71-NOT: warning:
 // CHECK-MCPU-DEFAULT-AIX71: {{.*}}clang{{.*}}" "-cc1"
-// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr4"
+// CHECK-MCPU-DEFAULT-AIX71: "-target-cpu" "pwr7"
 
 // Check that the user is able to overwrite the default with '-mcpu'.
 // RUN: %clang -no-canonical-prefixes %s -### -c 2>&1 \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -403,14 +403,9 @@
 if (!TargetCPUName.empty())
   return TargetCPUName;
 
-if (T.isOSAIX()) {
-  unsigned major, minor, unused_micro;
-  T.getOSVersion(major, minor, unused_micro);
-  // The minimal arch level moved from pwr4 for AIX7.1 to
-  // pwr7 for AIX7.2.
-  TargetCPUName =
-  (major < 7 || (major == 7 && minor < 2)) ? "pwr4" : "pwr7";
-} else if (T.getArch() == llvm::Triple::ppc64le)
+if (T.isOSAIX())
+  TargetCPUName = "pwr7";
+else if (T.getArch() == llvm::Triple::ppc64le)
   TargetCPUName = "ppc64le";
 else if (T.getArch() == llvm::Triple::ppc64)
   TargetCPUName = "ppc64";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

@crownyanguan Why do you think this caused the linker to crash?  It is a 
warning in the front end.  Would it even be executed in your posted command?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9cb00b9ecbe7: Reland Produce warning for performing pointer 
arithmetic on a null pointer. (authored by jamieschmeiser).

Changed prior to commit:
  https://reviews.llvm.org/D98798?vs=346751&id=360116#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/Inputs/pointer-subtraction.h
  clang/test/Sema/pointer-subtraction.c
  clang/test/Sema/pointer-subtraction.cpp

Index: clang/test/Sema/pointer-subtraction.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char *)0;
+  f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char *)((char *)0 - (char *)0); // valid in C++
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+#endif
+}
Index: clang/test/Sema/pointer-subtraction.c
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char *)0;
+  f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char *)((char *)0 - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+#endif
+}
Index: clang/test/Sema/Inputs/pointer-subtraction.h
===
--- /dev/null
+++ clang/test/Sema/Inputs/pointer-subtraction.h
@@ -0,0 +1 @@
+#define SYSTEM_MACRO(x) (x) = (char *)((char *)0 - (x))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10432,6 +10432,22 @@
   << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
 }
 
+/// Diagnose invalid subraction on a null pointer.
+///
+static void diagnoseSubtractionOnNullPointer(Sema &S, SourceLocation Loc,
+ Expr *Pointer, bool BothNull) {
+  // Null - null is valid in C++ [expr.add]p7
+  if (BothNull && S.getLangOpts().CPlusPlus)
+return;
+
+  // Is this s a macro from a system header?
+  if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
+return;
+
+  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
+  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+}
+
 /// Diagnose invalid arithmetic on two function pointers.
 static void diagnoseArithmeticOnTwoFunctionPointers(Sema &S, SourceLocation Loc,
 Expr *LHS, Expr *RHS) {
@@ -10863,7 +10879,16 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->Ignor

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

This was originally approved and landed on May 11th.  I agreed to let it be 
reverted when it was discovered that some headers were triggering the warning.  
I reworked the code to not generate the warning when coming from system header 
files and also added option control for the warnings.  I was informed that the 
changes fixed the problems with the system headers but have received no other 
feedback since late May, despite numerous pings and requests tagging the 
various people involved.  Since I have received no objections, further comments 
nor further review in approximately 2 months, I am assuming that there are no 
further concerns.  Unless I hear otherwise, I will commit these changes 
tomorrow.


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

https://reviews.llvm.org/D98798

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

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG73840f9f8141: thread_local support for AIX (authored by 
jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -1,19 +1,20 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefixes=CHECK,AIX,LINUX_AIX %s
 
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
 int f();
 int g();
 
-// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -23,7 +24,7 @@
 static thread_local int d = g();
 
 struct U { static thread_local int m; };
-// LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -89,9 +90,9 @@
 
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
-// LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
-// LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +107,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} 

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 359336.
jamieschmeiser added a comment.

Respond to review comments:  Change parameter name and tighten up conditions.


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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -1,19 +1,20 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefixes=CHECK,AIX,LINUX_AIX %s
 
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
 int f();
 int g();
 
-// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -23,7 +24,7 @@
 static thread_local int d = g();
 
 struct U { static thread_local int m; };
-// LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -89,9 +90,9 @@
 
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
-// LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
-// LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +107,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CALL]], i32* @_ZZ1fvE1n, align 4
   // CHECK: store i8 1, 

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 358974.
jamieschmeiser added a comment.

Respond to review comments, formatting: fix comments, add assertion, fix
linkage, expand test to include body of generated function.


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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -1,19 +1,20 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefixes=CHECK,AIX,LINUX_AIX %s
 
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
 int f();
 int g();
 
-// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -23,7 +24,7 @@
 static thread_local int d = g();
 
 struct U { static thread_local int m; };
-// LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -89,9 +90,9 @@
 
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
-// LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
-// LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +107,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CAL

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 358717.
jamieschmeiser added a comment.

Remove accidental inclusion


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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -1,19 +1,20 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefixes=CHECK,AIX,LINUX_AIX %s
 
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
 int f();
 int g();
 
-// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -23,7 +24,7 @@
 static thread_local int d = g();
 
 struct U { static thread_local int m; };
-// LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -89,9 +90,9 @@
 
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
-// LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
-// LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +107,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CALL]], i32* @_ZZ1fvE1n, align 4
   // CHECK: store i8 1, i8* @_ZGVZ1fvE1n
   // CHECK: br label
@@ -126,55 

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2971
+isEmittedWithConstantInitializer(VD, true) &&
+!VD->needsDestruction(getContext())) {
+  // Emit a weak global function referring to the initialization function.

hubert.reinterpretcast wrote:
> Unfortunately `needsDestruction` cannot be counted on at this time for some 
> incomplete types, see https://llvm.org/pr51079.
I think it is okay to leave the code as-is as it will then be fixed when that 
problem is fixed.


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

https://reviews.llvm.org/D104420

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

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 358690.
jamieschmeiser added a comment.

Respond to review comments: update comments and make functions not weak


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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -1,19 +1,20 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefixes=CHECK,AIX,LINUX_AIX %s
 
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
 int f();
 int g();
 
-// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -23,7 +24,7 @@
 static thread_local int d = g();
 
 struct U { static thread_local int m; };
-// LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -89,9 +90,9 @@
 
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
-// LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
-// LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +107,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CALL]], i32* @_ZZ1fvE1n, align 4
   // CHECK: store i8 1, i8* @_

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 358673.
jamieschmeiser added a comment.

Respond to review comments: create stub functions, return 0 from function and 
other fixes


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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -1,19 +1,20 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefixes=CHECK,AIX,LINUX_AIX %s
 
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
 int f();
 int g();
 
-// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -23,7 +24,7 @@
 static thread_local int d = g();
 
 struct U { static thread_local int m; };
-// LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -89,9 +90,9 @@
 
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
-// LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
-// LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +107,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CALL]], i32* @_ZZ1fvE1n, align 4
   // CHECK:

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D98798

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

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 357970.
jamieschmeiser added a comment.

Respond to review comments:  Update to tests to create common LINUX/AIX 
portions.


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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -1,19 +1,20 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefixes=CHECK,AIX,LINUX_AIX %s
 
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
-// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX --check-prefix=CHECK-OPT %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
+// RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -O2 -disable-llvm-passes -o - -triple x86_64-linux-gnu | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX,CHECK-OPT %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
-// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefixes=CHECK,LINUX,LINUX_AIX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
 int f();
 int g();
 
-// LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -23,7 +24,7 @@
 static thread_local int d = g();
 
 struct U { static thread_local int m; };
-// LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// LINUX_AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -89,9 +90,9 @@
 
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
-// LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
-// LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// LINUX_AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +107,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CALL]], i32* @_ZZ1fvE1n, align 4
   // CHECK: store i8 1, i8* @_ZGVZ1fvE1n
   // CHECK: br label
@@ -126,55 +127,57 @@
 }
 
 

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 356705.
jamieschmeiser added a comment.

Fix formatting problem.


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

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -9,11 +9,13 @@
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
 // RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefix=CHECK --check-prefix=AIX %s
 
 int f();
 int g();
 
 // LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -24,6 +26,7 @@
 
 struct U { static thread_local int m; };
 // LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -90,8 +93,10 @@
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
 // LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
 // LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +111,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CALL]], i32* @_ZZ1fvE1n, align 4
   // CHECK: store i8 1, i8* @_ZGVZ1fvE1n
   // CHECK: br label
@@ -127,54 +132,67 @@
 
 // CHECK: define {{.*}} @[[C_INIT:.*]]()
 // LINUX: call i32* @_ZTW1b()
+// AIX: call i32* @_ZTW1b()
 // DARWIN: call cxx_fast_tlscc i32* @_ZTW1b()
 // CHECK-NEXT: load i32, i32* %{{.*}}, align 4
 // CHECK-NEXT: store i32 %{{.*}}, i32* @c, align 4
 
 // LINUX-LABEL: define linkonce_odr hidden i32* @_ZTW1b()
+// AIX-LABEL: define linkonce_odr hidden i32* @_ZTW1b()
 // LINUX: br i1 icmp ne (void ()* @_ZTH1b, void ()* null),
 // not null:
 // LINUX: call void @_ZTH1b()
+// AIX-NOT: br i1 icmp ne (void ()* @_ZTH1b, void ()* null),
+// AIX: call void @_ZTH1b()
 // LINUX: br label
 // finally:
 // LINUX: ret i32* @b
+// AIX: ret i32* @b
 // DARWIN-LABEL: declare cxx_fast_tlscc i32* @_ZTW1b()
 // There is no definition of the thread wrapper on Darwin for external TLV.
 
 // CHECK: define {{.*}} @[[D_INIT:.*]]()
-// CHECK: call i32 @_Z1gv()
+// CHECK: call{{.*}} i32 @_Z1gv()
 // CHECK-NEXT: store i32 %{{.*}}, i32* @_ZL1d, align 4
 
 // CHECK: define {{.*}} @[[U_M_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 %{{.*}}, i32* @_ZN1U1mE, align 4
 
 // CHECK: define {{.*}} @[[E_INIT:.*]]()
 // LINUX: call i32* @_ZTWN1VIiE1mE()
+// AIX: call i32* @_ZTWN1VIiE1mE()
 // DARWIN: call cxx_fast_tlscc i32* @_ZTWN1VIiE1mE()
 // CHECK-NEXT: load i32, i32* %{{.*}}, align 4
 // LINUX: call {{.*}}* @_ZTWN1XIiE1mE()
+// AIX: call {{.*}}* @_ZTWN1XIiE1mE()
 // DARWIN: call cxx_fast_tlscc {{.*}}* @_ZTWN1XIiE1mE()
 // CHECK: store {{.*}} @e
 
 // LINUX-LABEL: define weak_odr hidden i32* @_ZTWN1VIiE1mE()
+// AIX-LABEL: define weak_odr hidden i32* @_ZTWN1VIiE1mE()
 // DARWIN-LABEL: define weak_odr hidden cxx_fast_tlscc i32* @_ZTWN1VIiE1mE()
 // LINUX: call void @_ZTHN1VIiE1mE()
+// AIX: call void @_ZTHN1VIiE1mE()
 // DARWIN: call cxx_fast_tlscc void @_ZTHN1VIiE1mE()
 // CHECK: ret i32* @_ZN1VIiE1mE
 
 // LINUX-LABEL: define weak_odr hidden i32* @_ZTWN1WIiE1mE

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

@rsmith @thakis @efriedma  If there are no more changes required, please 
approve.


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

https://reviews.llvm.org/D98798

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

[PATCH] D104420: thread_local support for AIX

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added reviewers: hubert.reinterpretcast, sfertile, 
cebowleratibm, DiggerLin, rsmith.
jamieschmeiser requested review of this revision.
Herald added a project: clang.

Changes to support thread_local storage on AIX.

  

The AIX linker will produce errors on unresolved weak symbols.  Change the
generated code to not check for the initialization function but just call
it and ensure that it always exists.  Also, the AIX atexit routine has a
different name (and signature) so call it correctly.  Update the lit tests
to test on AIX appropriately.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104420

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -9,11 +9,13 @@
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -femulated-tls -emit-llvm %s -o - \
 // RUN: -triple x86_64-linux-gnu 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
 // RUN: %clang_cc1 -std=c++11 -fno-use-cxa-atexit -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple powerpc64-unknown-aix-xcoff | FileCheck --check-prefix=CHECK --check-prefix=AIX %s
 
 int f();
 int g();
 
 // LINUX-DAG: @a ={{.*}} thread_local global i32 0
+// AIX-DAG: @a ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @a = internal thread_local global i32 0
 thread_local int a = f();
 extern thread_local int b;
@@ -24,6 +26,7 @@
 
 struct U { static thread_local int m; };
 // LINUX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
+// AIX-DAG: @_ZN1U1mE ={{.*}} thread_local global i32 0
 // DARWIN-DAG: @_ZN1U1mE = internal thread_local global i32 0
 thread_local int U::m = f();
 
@@ -90,8 +93,10 @@
 // CHECK-DAG: @llvm.global_ctors = appending global {{.*}} @[[GLOBAL_INIT:[^ ]*]]
 
 // LINUX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
+// AIX-DAG: @_ZTH1a ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTH1a = internal alias void (), void ()* @__tls_init
 // LINUX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
+// AIX-DAG: @_ZTHN1U1mE ={{.*}} alias void (), void ()* @__tls_init
 // DARWIN-DAG: @_ZTHN1U1mE = internal alias void (), void ()* @__tls_init
 // CHECK-DAG: @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @[[V_M_INIT:[^, ]*]]
 // CHECK-DAG: @_ZTHN1XIiE1mE = linkonce_odr alias void (), void ()* @[[X_M_INIT:[^, ]*]]
@@ -106,16 +111,16 @@
 // Individual variable initialization functions:
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 {{.*}}, i32* @a, align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
 int f() {
   // CHECK: %[[GUARD:.*]] = load i8, i8* @_ZGVZ1fvE1n, align 1
   // CHECK: %[[NEED_INIT:.*]] = icmp eq i8 %[[GUARD]], 0
-  // CHECK: br i1 %[[NEED_INIT]]
+  // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
-  // CHECK: %[[CALL:.*]] = call i32 @_Z1gv()
+  // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
   // CHECK: store i32 %[[CALL]], i32* @_ZZ1fvE1n, align 4
   // CHECK: store i8 1, i8* @_ZGVZ1fvE1n
   // CHECK: br label
@@ -127,54 +132,67 @@
 
 // CHECK: define {{.*}} @[[C_INIT:.*]]()
 // LINUX: call i32* @_ZTW1b()
+// AIX: call i32* @_ZTW1b()
 // DARWIN: call cxx_fast_tlscc i32* @_ZTW1b()
 // CHECK-NEXT: load i32, i32* %{{.*}}, align 4
 // CHECK-NEXT: store i32 %{{.*}}, i32* @c, align 4
 
 // LINUX-LABEL: define linkonce_odr hidden i32* @_ZTW1b()
+// AIX-LABEL: define linkonce_odr hidden i32* @_ZTW1b()
 // LINUX: br i1 icmp ne (void ()* @_ZTH1b, void ()* null),
 // not null:
 // LINUX: call void @_ZTH1b()
+// AIX-NOT: br i1 icmp ne (void ()* @_ZTH1b, void ()* null),
+// AIX: call void @_ZTH1b()
 // LINUX: br label
 // finally:
 // LINUX: ret i32* @b
+// AIX: ret i32* @b
 // DARWIN-LABEL: declare cxx_fast_tlscc i32* @_ZTW1b()
 // There is no definition of the thread wrapper on Darwin for external TLV.
 
 // CHECK: define {{.*}} @[[D_INIT:.*]]()
-// CHECK: call i32 @_Z1gv()
+// CHECK: call{{.*}} i32 @_Z1gv()
 // CHECK-NEXT: store i32 %{{.*}}, i32* @_ZL1d, align 4
 
 // CHECK: define {{.*}} @[[U_M_INIT:.*]]()
-// CHECK: call i32 @_Z1fv()
+// CHECK: call{{.*}} i32 @_Z1fv()
 // CHECK-NEXT: store i32 %{{.*}}, i32* @_ZN1U1mE, align 4
 
 // CHECK: define {{.*}} @[[E_INIT:.*]]()
 // LINUX: call i32* @_ZTWN1VIiE1mE()
+// AIX: call i32* @_ZTWN1VIiE1mE()
 // DARWIN: call cxx_fast_tlscc i32* @_ZTWN1VIiE1mE()
 // CHECK-NEXT: load i32, i32* %{{.*}}, align 4
 // LINUX: call {{.*}}* @_ZTWN1XIiE1mE()
+// AIX: call {{.*}}* @_ZTWN1XIiE1mE()
 // DARWIN: call cxx_fast_tlscc {{.*}}* @

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

@rsmith I separated the C and C++ messages in response to your comments and 
changed the C++ message to state that "performing pointer subtraction with a 
null pointer may have undefined behavior" to address your concerns.  Are you 
satisfied?  @thakis, the warning no longer fires in the MS headers according to 
@hans and there is separate option control over this warning.  Is this 
sufficient?  @efriedma, thank you for reviewing/approving the original but 
these changes were sufficiently different that I thought a new review would be 
beneficial.  Are you still satisfied with the resulting changes?


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

The reason I worded it with 'may' is because, in C++, nullptr - nullptr is 
defined.  If the code is "nullptr - p" or "p - nullptr", it is only undefined 
behaviour when p is not nullptr, hence the 'may' part of the warning because 
this is not known at compile time.  The warning is still useful as it is 
suspect code but one cannot state that it is undefined behaviour because it is 
valid if it is null at runtime.


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a reviewer: thakis.
jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

@thakis, can you please verify that the changes no longer affect the MS 
headers?  Thanks.


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

https://reviews.llvm.org/D98798

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

[PATCH] D100991: Fix parsing of vector keyword in presence of conflicting uses.

[Jamie Schmeiser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG136ced498ba8: When vector is found as a type or non-type id, 
check if it is really the… (authored by jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100991

Files:
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/altivec-non-type-vector.c
  clang/test/Parser/altivec-template-vector.cpp
  clang/test/Parser/altivec-typedef-vector.c


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-linux-gnu
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-linux-gnu
+
+template  class vector {
+public:
+  vector(int) {}
+};
+
+void f() {
+  vector int v = {0};
+  vector vi = {0};
+}
Index: clang/test/Parser/altivec-non-type-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-non-type-vector.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-linux-gnu
+
+int vector();
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1695,6 +1695,11 @@
 break;
 
   case Sema::NC_Type: {
+if (TryAltiVecVectorToken())
+  // vector has been found as a type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 SourceLocation BeginLoc = NameLoc;
 if (SS.isNotEmpty())
   BeginLoc = SS.getBeginLoc();
@@ -1736,6 +1741,11 @@
 return ANK_Success;
 
   case Sema::NC_NonType:
+if (TryAltiVecVectorToken())
+  // vector has been found as a non-type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 Tok.setKind(tok::annot_non_type);
 setNonTypeAnnotation(Tok, Classification.getNonTypeDecl());
 Tok.setLocation(NameLoc);


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-linux-gnu
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altive

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 346751.
jamieschmeiser added a comment.

Fix formatting.


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

https://reviews.llvm.org/D98798

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/Inputs/pointer-subtraction.h
  clang/test/Sema/pointer-subtraction.c
  clang/test/Sema/pointer-subtraction.cpp

Index: clang/test/Sema/pointer-subtraction.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char *)0;
+  f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char *)((char *)0 - (char *)0); // valid in C++
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+#endif
+}
Index: clang/test/Sema/pointer-subtraction.c
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char *)0;
+  f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char *)((char *)0 - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+#endif
+}
Index: clang/test/Sema/Inputs/pointer-subtraction.h
===
--- /dev/null
+++ clang/test/Sema/Inputs/pointer-subtraction.h
@@ -0,0 +1 @@
+#define SYSTEM_MACRO(x) (x) = (char *)((char *)0 - (x))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10366,6 +10366,22 @@
   << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
 }
 
+/// Diagnose invalid subraction on a null pointer.
+///
+static void diagnoseSubtractionOnNullPointer(Sema &S, SourceLocation Loc,
+ Expr *Pointer, bool BothNull) {
+  // Null - null is valid in C++ [expr.add]p7
+  if (BothNull && S.getLangOpts().CPlusPlus)
+return;
+
+  // Is this s a macro from a system header?
+  if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
+return;
+
+  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
+  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+}
+
 /// Diagnose invalid arithmetic on two function pointers.
 static void diagnoseArithmeticOnTwoFunctionPointers(Sema &S, SourceLocation Loc,
 Expr *LHS, Expr *RHS) {
@@ -10797,7 +10813,16 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+  bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+
+  // Subtracting nullptr or from nullptr is suspect
+  if (LHSIsNullPtr)
+ 

[PATCH] D100991: Fix parsing of vector keyword in presence of conflicting uses.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 346722.
jamieschmeiser added a comment.

Remove unnecessary tests.


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

https://reviews.llvm.org/D100991

Files:
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/altivec-non-type-vector.c
  clang/test/Parser/altivec-template-vector.cpp
  clang/test/Parser/altivec-typedef-vector.c


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-linux-gnu
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-linux-gnu
+
+template  class vector {
+public:
+  vector(int) {}
+};
+
+void f() {
+  vector int v = {0};
+  vector vi = {0};
+}
Index: clang/test/Parser/altivec-non-type-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-non-type-vector.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple 
powerpc-linux-gnu
+
+int vector();
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1695,6 +1695,11 @@
 break;
 
   case Sema::NC_Type: {
+if (TryAltiVecVectorToken())
+  // vector has been found as a type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 SourceLocation BeginLoc = NameLoc;
 if (SS.isNotEmpty())
   BeginLoc = SS.getBeginLoc();
@@ -1736,6 +1741,11 @@
 return ANK_Success;
 
   case Sema::NC_NonType:
+if (TryAltiVecVectorToken())
+  // vector has been found as a non-type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 Tok.setKind(tok::annot_non_type);
 setNonTypeAnnotation(Tok, Classification.getNonTypeDecl());
 Tok.setLocation(NameLoc);


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-linux-gnu
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-ibm-aix-xcoff

[PATCH] D100991: Fix parsing of vector keyword in presence of conflicting uses.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 346548.
jamieschmeiser added a comment.

Expand testing.


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

https://reviews.llvm.org/D100991

Files:
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/altivec-non-type-vector.c
  clang/test/Parser/altivec-template-vector.cpp
  clang/test/Parser/altivec-typedef-vector.c

Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-linux-unknown
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-unknown-linux
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-unknown-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-unknown-unknown
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux-musl
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-montavista-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-none-none
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-aix
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-unknown
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-ibm-aix
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-unknown-aix
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-unknown-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-unknown-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-unknown-openbsd
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-unknown-unknown
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-linux-unknown
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-unknown-linux
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-unknown-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64le-unknown-unknown
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-linux-musl
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-montavista-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-none-none
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-aix
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc64-unknown-unknown
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-ibm-aix
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-ibm-aix-xcoff
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-linux-gnu
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s -triple powerpc-unknown-aix
+// RUN: %clang_cc1 -target-featu

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser requested review of this revision.
jamieschmeiser added a comment.

Significant changes made since previously accepted.


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 346515.
jamieschmeiser edited the summary of this revision.
jamieschmeiser added a comment.

As requested, I have added a new warning option -Wnull-pointer-subtraction (and 
added it to -Wextra) that does not trigger on system headers.  In addition, I 
changed the message used such that it states that it is undefined behaviour for 
C but may be undefined behaviour in C++ to address the concerns that performing 
the subtraction with a pointer that dynamically becomes null is not undefined 
behaviour.  Expanded the testing to also test that the warning does not fire on 
system headers.  Also, updated the release notes to indicate the new option.


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

https://reviews.llvm.org/D98798

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/Inputs/pointer-subtraction.h
  clang/test/Sema/pointer-subtraction.c
  clang/test/Sema/pointer-subtraction.cpp

Index: clang/test/Sema/pointer-subtraction.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // valid in C++
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+#endif
+}
Index: clang/test/Sema/pointer-subtraction.c
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+#endif
+}
Index: clang/test/Sema/Inputs/pointer-subtraction.h
===
--- /dev/null
+++ clang/test/Sema/Inputs/pointer-subtraction.h
@@ -0,0 +1 @@
+#define SYSTEM_MACRO(x) (x) = (char*)((char*)0 - (x))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10366,6 +10366,22 @@
   << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
 }
 
+/// Diagnose invalid subraction on a null pointer.
+///
+static void diagnoseSubtractionOnNullPointer(Sema &S, SourceLocation Loc,
+	 Expr *Pointer, bool BothNull) {
+  // Null - null is valid in C++ [expr.add]p7
+  if (BothNull && S.getLangOpts().CPlusPlus)
+return;
+
+  // Is this s a macro from a system header?
+  if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
+return;
+
+  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+}
+
 /// Diagnose invalid arithmetic on two function pointers.
 static void diagnoseArithmeticOnTwoFunctionPointers(Sema &S, SourceLocation Loc,
 Expr *LHS, Expr *RHS) {
@@ -10797,7 +10813,16 @@
  

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser reopened this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

Re-opening because it was reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

If this is a problem for you, please revert it and I will take a look when I 
return.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

@thakis, I have some questions about your comments.
Are you sure this is coming from a system header?  The path that you gave has 
third_party as a directory in the path.  If the warning were being triggered by 
code in a system header, I would have expected it to fire on something in the 
windows build bots but they appear to be clean.
I don't know if it is possible to suppress a warning based on whether the 
source is in a system header, or from a macro expansion that is defined in a 
system header; are you aware about whether or not this is possible?  If it is, 
I suspect it would already be in force as a general setting, again making me 
question whether this is a system header...and if it isn't a system header, 
that wouldn't help you in any case.
If I understand your second point correctly, you have a system that previously 
compiled cleanly with warnings being treated as errors and you are concerned 
that if you use the existing options to suppress this particular warning, you 
are concerned that something could creep in.  Therefore you are proposing a 
different warning group that would be a subgroup of the existing one so that if 
you set it, you would set both but you would still be able to set the specific 
one.  I don't know if this is possible or not.  Either way, I suspect that it 
would require using a different warning for the subtraction case, which would 
also require significantly changing these changes. Am I understanding 
correctly?  If so, this implies that you have access to a workaround for your 
problem, although it may not be the best solution.
I do not have access to a windows setup to test any of these proposed changes; 
in particular, given that I suspect that the affected files are specific to 
some third party vendor from which you have purchased code, I do not have means 
of investigating the actual problems/solutions.  If it would be helpful, I 
would be happy to review any changes that you might like to make to remedy the 
situation.
I will be on vacation for the next few days so please excuse my delayed 
responses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D100991: Fix parsing of vector keyword in presence of conflicting uses.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D100991

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdfc1e31d49fe: Produce warning for performing pointer 
arithmetic on a null pointer. (authored by jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c
  clang/test/Sema/pointer-addition.cpp


Index: clang/test/Sema/pointer-addition.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-addition.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // valid in C++
+}
Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}} expected-warning 
{{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10779,7 +10779,17 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+  bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+
+  // Subtracting nullptr or from nullptr should produce
+  // a warning expect nullptr - nullptr is valid in C++ [expr.add]p7
+  if (LHSIsNullPtr && (!getLangOpts().CPlusPlus || !RHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+  if (RHSIsNullPtr && (!getLangOpts().CPlusPlus || !LHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-addition.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // valid in C++
+}
Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 343483.
jamieschmeiser added a comment.

Respond to review comments: add C++ test.


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

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c
  clang/test/Sema/pointer-addition.cpp


Index: clang/test/Sema/pointer-addition.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-addition.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // valid in C++
+}
Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}} expected-warning 
{{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10814,7 +10814,17 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+  bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+
+  // Subtracting nullptr or from nullptr should produce
+  // a warning expect nullptr - nullptr is valid in C++ [expr.add]p7
+  if (LHSIsNullPtr && (!getLangOpts().CPlusPlus || !RHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+  if (RHSIsNullPtr && (!getLangOpts().CPlusPlus || !LHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-addition.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // valid in C++
+}
Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10814,7 +10814,17 @@
LHS.get(), R

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D98798

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

[PATCH] D100991: Fix parsing of vector keyword in presence of conflicting uses.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 342540.
jamieschmeiser added a comment.

Limit tests to platform that supports altivec.


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

https://reviews.llvm.org/D100991

Files:
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/altivec-non-type-vector.c
  clang/test/Parser/altivec-template-vector.cpp
  clang/test/Parser/altivec-typedef-vector.c


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix-xcoff -target-feature 
+altivec -fsyntax-only %s
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix-xcoff -target-feature 
+altivec -fsyntax-only %s
+
+template  class vector {
+public:
+  vector(int) {}
+};
+
+void f() {
+  vector int v = {0};
+  vector vi = {0};
+}
Index: clang/test/Parser/altivec-non-type-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-non-type-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix-xcoff -target-feature 
+altivec -fsyntax-only %s
+
+int vector();
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1695,6 +1695,11 @@
 break;
 
   case Sema::NC_Type: {
+if (TryAltiVecVectorToken())
+  // vector has been found as a type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 SourceLocation BeginLoc = NameLoc;
 if (SS.isNotEmpty())
   BeginLoc = SS.getBeginLoc();
@@ -1736,6 +1741,11 @@
 return ANK_Success;
 
   case Sema::NC_NonType:
+if (TryAltiVecVectorToken())
+  // vector has been found as a non-type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 Tok.setKind(tok::annot_non_type);
 setNonTypeAnnotation(Tok, Classification.getNonTypeDecl());
 Tok.setLocation(NameLoc);


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix-xcoff -target-feature +altivec -fsyntax-only %s
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix-xcoff -target-feature +altivec -fsyntax-only %s
+
+template  class vector {
+public:
+  vector(int) {}
+};
+
+void f() {
+  vector int v = {0};
+  vector vi = {0};
+}
Index: clang/test/Parser/altivec-non-type-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-non-type-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix-xcoff -target-feature +altivec -fsyntax-only %s
+
+int vector();
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1695,6 +1695,11 @@
 break;
 
   case Sema::NC_Type: {
+if (TryAltiVecVectorToken())
+  // vector has been found as a type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 SourceLocation BeginLoc = NameLoc;
 if (SS.isNotEmpty())
   BeginLoc = SS.getBeginLoc();
@@ -1736,6 +1741,11 @@
 return ANK_Success;
 
   case Sema::NC_NonType:
+if (TryAltiVecVectorToken())
+  // vector has been found as a non-type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 Tok.setKind(tok::annot_non_type);
 setNonTypeAnnotation(Tok, Classification.getNonTypeDecl());
 Tok.setLocation(NameLoc);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

ping


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

https://reviews.llvm.org/D98798

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

[PATCH] D100991: Fix parsing of vector keyword in presence of conflicting uses.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added reviewers: rsmith, saar.raz.
jamieschmeiser requested review of this revision.
Herald added a project: clang.

When vector is found as a type or non-type id, check if it is really the 
altivec vector token.

Call TryAltiVecVectorToken when an identifier is seen in the parser before
annotating the token.  This checks the next token where necessary to ensure
that vector is properly handled as the altivec token.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100991

Files:
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/altivec-non-type-vector.c
  clang/test/Parser/altivec-template-vector.cpp
  clang/test/Parser/altivec-typedef-vector.c


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -target-feature +altivec %s
+
+template  class vector {
+public:
+  vector(int) {}
+};
+
+void f() {
+  vector int v = {0};
+  vector vi = {0};
+}
Index: clang/test/Parser/altivec-non-type-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-non-type-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s
+
+int vector();
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1695,6 +1695,11 @@
 break;
 
   case Sema::NC_Type: {
+if (TryAltiVecVectorToken())
+  // vector has been found as a type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 SourceLocation BeginLoc = NameLoc;
 if (SS.isNotEmpty())
   BeginLoc = SS.getBeginLoc();
@@ -1736,6 +1741,11 @@
 return ANK_Success;
 
   case Sema::NC_NonType:
+if (TryAltiVecVectorToken())
+  // vector has been found as a non-type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 Tok.setKind(tok::annot_non_type);
 setNonTypeAnnotation(Tok, Classification.getNonTypeDecl());
 Tok.setLocation(NameLoc);


Index: clang/test/Parser/altivec-typedef-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-typedef-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s
+
+typedef int vector;
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/test/Parser/altivec-template-vector.cpp
===
--- /dev/null
+++ clang/test/Parser/altivec-template-vector.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -target-feature +altivec %s
+
+template  class vector {
+public:
+  vector(int) {}
+};
+
+void f() {
+  vector int v = {0};
+  vector vi = {0};
+}
Index: clang/test/Parser/altivec-non-type-vector.c
===
--- /dev/null
+++ clang/test/Parser/altivec-non-type-vector.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -target-feature +altivec -fsyntax-only %s
+
+int vector();
+
+void test() {
+  vector unsigned int v = {0};
+}
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1695,6 +1695,11 @@
 break;
 
   case Sema::NC_Type: {
+if (TryAltiVecVectorToken())
+  // vector has been found as a type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 SourceLocation BeginLoc = NameLoc;
 if (SS.isNotEmpty())
   BeginLoc = SS.getBeginLoc();
@@ -1736,6 +1741,11 @@
 return ANK_Success;
 
   case Sema::NC_NonType:
+if (TryAltiVecVectorToken())
+  // vector has been found as a non-type id when altivec is enabled but
+  // this is followed by a declaration specifier so this is really the
+  // altivec vector token.  Leave it unannotated.
+  break;
 Tok.setKind(tok::annot_non_type);
 setNonTypeAnnotation(Tok, Classification.getNonTypeDecl());
 Tok.setLocation(NameLoc);
___
cfe-commits mailing list
cfe-commits@lists.ll

[PATCH] D100231: [NewPM] Cleanup IR printing instrumentation

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser accepted this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100231

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 336828.
jamieschmeiser added a comment.

Fix indenting.


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

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}} expected-warning 
{{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,17 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+  bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+
+  // Subtracting nullptr or from nullptr should produce
+  // a warning expect nullptr - nullptr is valid in C++ [expr.add]p7
+  if (LHSIsNullPtr && (!getLangOpts().CPlusPlus || !RHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+  if (RHSIsNullPtr && (!getLangOpts().CPlusPlus || !LHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,17 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+  bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+
+  // Subtracting nullptr or from nullptr should produce
+  // a warning expect nullptr - nullptr is valid in C++ [expr.add]p7
+  if (LHSIsNullPtr && (!getLangOpts().CPlusPlus || !RHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+  if (RHSIsNullPtr && (!getLangOpts().CPlusPlus || !LHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 336567.
jamieschmeiser added a comment.

Respond to review comments:  Do not issue warning for nullptr - nullptr in C++.


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

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}} expected-warning 
{{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,17 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+  bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+
+  // Subtracting nullptr or from nullptr should produce
+  // a warning expect nullptr - nullptr is valid in C++ [expr.add]p7
+  if (LHSIsNullPtr && (!getLangOpts().CPlusPlus || !RHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+  if (RHSIsNullPtr && (!getLangOpts().CPlusPlus || !LHSIsNullPtr))
+  diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,17 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+  bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull);
+
+  // Subtracting nullptr or from nullptr should produce
+  // a warning expect nullptr - nullptr is valid in C++ [expr.add]p7
+  if (LHSIsNullPtr && (!getLangOpts().CPlusPlus || !RHSIsNullPtr))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+  if (RHSIsNullPtr && (!getLangOpts().CPlusPlus || !LHSIsNullPtr))
+  diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added inline comments.



Comment at: clang/test/Sema/pointer-addition.c:34
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}} expected-warning 
{{performing pointer arithmetic on a null pointer has undefined behavior}}
 }

jamieschmeiser wrote:
> efriedma wrote:
> > This is what I was afraid would happen.
> > 
> > C++ has a specific carveout in the standard that "null-null" isn't 
> > undefined. C doesn't, but I'm not sure warning is practically useful there.
> > 
> > In any case, printing the same warning twice isn't useful.
> Yes, I see that [expr.add] p 7 says nullptr-nullptr is defined to be 0.  I 
> will suppress the warning for this.
I disagree with the comment that the two warnings are not useful.  While it is 
the same warning, the section of code indicated by each warning is different 
and both need to be corrected to clear the code of warnings.  A single warning 
would likely be more confusing in that a user would see the same warning and 
not notice that a different section of code is indicated after fixing the 
indicated problem.  I would expect the typical response to be the user assuming 
that they had made a mistake and removing the initial fix and fixing the 
second, only to again receive a same warning again with a different section of 
code.  The different code sections indicated would likely be overlooked, 
leading to frustration whereas two warnings clearly indicates there are 2 
problems.


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added inline comments.



Comment at: clang/test/Sema/pointer-addition.c:34
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}} expected-warning 
{{performing pointer arithmetic on a null pointer has undefined behavior}}
 }

efriedma wrote:
> This is what I was afraid would happen.
> 
> C++ has a specific carveout in the standard that "null-null" isn't undefined. 
> C doesn't, but I'm not sure warning is practically useful there.
> 
> In any case, printing the same warning twice isn't useful.
Yes, I see that [expr.add] p 7 says nullptr-nullptr is defined to be 0.  I will 
suppress the warning for this.


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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 336500.
jamieschmeiser added a comment.

Respond to review comments: add requested test.


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

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}} expected-warning 
{{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,7 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}} expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser updated this revision to Diff 336148.
jamieschmeiser added a comment.

Reformat to satisfy clang-format


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

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,6 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,6 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(
+  Context, Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98798: Produce waring for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added reviewers: rsmith, efriedma, hfinkel.
jamieschmeiser added a comment.

Added more reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce waring for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I used formatting similar to the existing code, which is not what clang-format 
is expecting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce waring for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added a reviewer: andrew.w.kaylor.
jamieschmeiser requested review of this revision.
Herald added a project: clang.

Test and produce warning for subtracting a pointer from null or subtracting 
null from a pointer.  Reuse existing warning that this is undefined behaviour.  
Also add unit test for both warnings.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,6 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,6 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser accepted this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I previously saw unrelated changes showing up in the differences here but this 
no longer seems to be the case so that comment can be ignored.




Comment at: llvm/include/llvm/IR/PassInstrumentation.h:126
 
+  void addClassToPassName(StringRef ClassName, StringRef PassName);
+  StringRef getPassNameForClassName(StringRef ClassName);

Comments describing these functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I see you have made the requested changes (nit: clang-tidy complained about the 
capitalization of the function) but why are there so many other, unrelated 
changes shown?  Is there a problem with the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I agree that having the callbacks ask for the names is an improvement as it is 
cleaner and allows other callbacks to use this feature if desired.  Generally, 
things look good.  I have a couple of minor concerns mentioned in the code but 
I think it would be acceptable as is if you don't agree with what I've 
mentioned.




Comment at: llvm/lib/Passes/PassBuilder.cpp:449
+  // We currently only use these for --print-before/after.
+  if (PIC && (!printBeforePasses().empty() || !printAfterPasses().empty())) {
+#define MODULE_PASS(NAME, CREATE_PASS) 
\

The tests for the options being empty should be moved to a separate function to 
facilitate expanding this feature for use with other pass instrumentation 
callbacks.  Right now, this is buried in here and if another callback wished to 
also use this freature, it might be hard to find.   Pulling the test out into 
an aptly named function would make it easier to find and understand.



Comment at: llvm/lib/Passes/PassBuilder.cpp:467
+#include "PassRegistry.def"
+#ifndef NDEBUG
+for (const auto &P : printBeforePasses()) {

This will silently give no tracing in release mode if the pass name does not 
exist (ie the error would not be reported and there would be no output).  Is 
this because of efficiency concerns?  It will only look for the names that are 
in the option list, which would typically be empty.  Rather than walking the 
string map, you could have a local stringset, add the pass names into it in the 
macros above if checking will be done and then use the stringset for the error 
checking.  I think this would be more efficient than walking the stringmap for 
producing the errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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

[PATCH] D87216: [NewPM] Support --print-before/after in NPM

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser requested changes to this revision.
jamieschmeiser added a comment.
This revision now requires changes to proceed.

The changes are specific to -print-before and -print-after (which is the 
intended target and this work originated before -print-changed) but could the 
change be made universal?  I would rather see the existing StringRef passed to 
all the callbacks changed rather than adding the new StringRef to specific 
callbacks.  This way all passes benefit and are consistent.  In particular, I 
would like to see this change applied to -printChanged; this came up in the 
reviews for that PR.  If you would rather not make it universal, then at least 
change all of the callbacks to also receive the new StringRef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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