[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked 2 inline comments as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:222
 {StreamState::FEof, StreamState::FError, StreamState::NoError}}},
-  {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+  // Note: ftell sets errno only.
+  {{"ftell", 1},

Szelethus wrote:
> Szelethus wrote:
> > C'99 standard [[ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf 
> > | §7.19.9.4.3]], about the `ftell function`:
> > 
> > > If successful, the `ftell` function returns the current value of the file 
> > > position indicator for the stream. On failure, the `ftell` function 
> > > returns `-1L` and stores an implementation-defined positive value in 
> > > `errno`.
> > 
> > So we need an evalCall for this.
> I mean, this function can only fail if the stream itself is an erroneous or 
> closed state, right? So we can associate the -1 return value with the stream 
> being in that, and the other with the stream being opened.
The `ftell` is part of a later patch that is adding `ftell` (and probably other 
functions). Only the "PossibleErrors" was updated in this patch because its 
meaning was changed (it contains value even if only one error kind is possible, 
before it was used only if at least two errors are possible).
It is not sure how the file operations work, maybe the `ftell` needs access to 
some data that is not available at the moment (and can fail even if the stream 
was not OK). If the stream is already in failed state the question is: Do ftell 
fail (then the we can make it return -1) or it does not fail but gives an 
unusable value (then generate a checker warning and stop the analysis).



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:485
+State = bindInt(0, State, C, CE);
+State = State->set(StreamSym, StreamState::getOpened(Desc));
+C.addTransition(State);

Szelethus wrote:
> Isn't the state change redundant? We have a `preCall` to this function and we 
> assert this as well.
We need to clarify what file operations are valid if the stream is in failed 
state or in EOF state. Again the question is if the function will simply fail 
again or it does not fail but does wrong things, or it will work correctly. 
Currently the initial stream error state is not taken into account for `fread` 
and `fwrite`, this needs to be fixed. For example for `fread`: "If an error 
occurs, the resulting value of the file position indicator for the stream is 
indeterminate." So at least a next read or write or `ftell` can not work 
correct after this (or works but with unusable result).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78374



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

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D78457#1991288 , @xazax.hun wrote:

> > As turned out we don't even need a BugReporterVisitor for doing the 
> > crosscheck.
> >  We should simply use the constraints of the ErrorNode since that has all 
> > the necessary information.
>
> This should not be true. But we should definitely have a test case that 
> proves this. The main idea is that unused symbols can be garbage collected. 
> The problem is that the ErrorNode does not have any information about the 
> symbols that were garbage collected earlier. This is why we need the visitor.


If a symbol is unused and garbage collected then that is not part of the path 
constraint that leads to the ErrorNode, is it? So why should we care about 
constraints on an unused symbol?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D75169: [ARM] Enforcing calling convention for half-precision FP arguments and returns for big-endian AArch32

[Lucas Prates via Phabricator via cfe-commits]

pratlucas added a comment.
Herald added a subscriber: danielkiss.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75169



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 258674.
gamesh411 added a comment.

Update user documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto &E : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189
+  if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+if (const ElementRegion *TheElementRegion =
+MRegion->getAs()) {

f00kat wrote:
> f00kat wrote:
> > NoQ wrote:
> > > NoQ wrote:
> > > > The sequence of `FieldRegion`s and `ElementRegion`s on top of a base 
> > > > region may be arbitrary: `var.a[0].b[1][2].c.d[3]` etc.
> > > > 
> > > > I'd rather unwrap those regions one-by-one in a loop and look at the 
> > > > alignment of each layer.
> > > Alternatively, just decompose the whole region into base region and 
> > > offset and see if base region has the necessary alignment and the offset 
> > > is divisible by the necessary alignment.
> > > The sequence of FieldRegions and ElementRegions on top of a base region 
> > > may be arbitrary: var.a[0].b[1][2].c.d[3] etc.
> > 
> > But i think(hope) I already do this and even have tests for this cases. For 
> > example
> > ```void test22() {
> >   struct alignas(alignof(short)) Z {
> > char p;
> > char c[10];
> >   };
> > 
> >   struct Y {
> > char p;
> > Z b[10];
> >   };
> > 
> >   struct X {
> > Y a[10];
> >   } Xi; // expected-note {{'Xi' initialized here}}
> > 
> >   // ok. 2(X align) + 1 (offset Y.p) + 1(align Z.p to 'short') + 1(offset 
> > Z.p) + 3(index)
> >   ::new (&Xi.a[0].b[0].c[3]) long;
> > }```
> > 
> > Cases with multidimensional arrays will also be handled correctly because 
> > method 'TheElementRegion->getAsArrayOffset()' calculates the offset for 
> > multidimensional arrays
> > ```void testXX() {
> > struct Y {
> > char p;
> > char b[10][10];
> > };
> > 
> > struct X {
> > Y a[10];
> > } Xi;
> > 
> > ::new (&Xi.a[0].b[0][0]) long;
> > }```
> > 
> > I can explain the code below for ElementRegion if needed.
> ?
Yeah, the tests are convincing and I think that you are handling the regions 
well.

On the other hand, this code is getting really complex, we should make it 
easier to read and understand.
E.g. `FieldOffsetValue` should be explained more, is it the offset started from 
the the start address of the multidimensional array, or it is just the offset 
from one element's start address?
Also, you have two variables named as `Offset`. They are offsets from which 
starting address? Perhaps we should have in the comments a running example, 
maybe for `&Xi.a[0].b[0][0]`? I mean is `FieldOffseValue` is standing for `b` 
or for `a`?


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

https://reviews.llvm.org/D76996



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

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

One way to test this change would be to add a statistic that is bumped each 
time a path is refuted (a report to be refuted we need all of the paths 
refuted, so using refuted reports might not be fine-grained enough). We can 
test on some big projects if the refuted paths increase or decrease after the 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D78478: [UpdateTestChecks] Add UTC_ARGS support for update_{llc,cc}_test_checks.py

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: jdoerfert, MaskRay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://reviews.llvm.org/D69701 added support for on-the-fly argument
changes for update scripts. I recently wanted to keep some manual check
lines in a test generated by update_cc_test_checks.py in our CHERI fork, so
this commit adds support for UTC_ARGS in update_cc_test_checks.py. And since
I was refactoring the code to be in common.py, I also added it for
update_llc_test_checks.py.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78478

Files:
  clang/test/utils/update_cc_test_checks/Inputs/mangled_names.c.funcsig.expected
  clang/test/utils/update_cc_test_checks/Inputs/on_the_fly_arg_change.c
  clang/test/utils/update_cc_test_checks/Inputs/on_the_fly_arg_change.c.expected
  clang/test/utils/update_cc_test_checks/mangled_names.test
  clang/test/utils/update_cc_test_checks/on_the_fly_arg_change.test
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/basic.ll.expected
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/on_the_fly_arg_change.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/on_the_fly_arg_change.ll.expected
  llvm/test/tools/UpdateTestChecks/update_llc_test_checks/basic.test
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/on_the_fly_arg_change.test
  llvm/utils/UpdateTestChecks/common.py
  llvm/utils/update_cc_test_checks.py
  llvm/utils/update_llc_test_checks.py
  llvm/utils/update_test_checks.py

Index: llvm/utils/update_test_checks.py
===
--- llvm/utils/update_test_checks.py
+++ llvm/utils/update_test_checks.py
@@ -32,18 +32,12 @@
 from __future__ import print_function
 
 import argparse
-import glob
-import itertools
-import os # Used to advertise this file's name ("autogenerated_note").
-import string
-import subprocess
-import sys
-import tempfile
+import os  # Used to advertise this file's name ("autogenerated_note").
 import re
+import sys
 
 from UpdateTestChecks import common
 
-ADVERT = '; NOTE: Assertions have been autogenerated by '
 
 def main():
   from argparse import RawTextHelpFormatter
@@ -58,58 +52,26 @@
   help='Keep function signature information around for the check line')
   parser.add_argument('--scrub-attributes', action='store_true',
   help='Remove attribute annotations (#0) from the end of check line')
-  parser.add_argument('--enable', action='store_true', dest='enabled', default=True,
-  help='Activate CHECK line generation from this point forward')
-  parser.add_argument('--disable', action='store_false', dest='enabled',
-  help='Deactivate CHECK line generation from this point forward')
   parser.add_argument('tests', nargs='+')
-  args = common.parse_commandline_args(parser)
+  initial_args = common.parse_commandline_args(parser)
 
   script_name = os.path.basename(__file__)
-  autogenerated_note = (ADVERT + 'utils/' + script_name)
-
-  opt_basename = os.path.basename(args.opt_binary)
+  opt_basename = os.path.basename(initial_args.opt_binary)
   if not re.match(r'^opt(-\d+)?$', opt_basename):
 common.error('Unexpected opt name: ' + opt_basename)
 sys.exit(1)
   opt_basename = 'opt'
 
-  for test in args.tests:
-if not glob.glob(test):
-  common.warn("Test file pattern '%s' was not found. Ignoring it." % (test,))
-  continue
-
-  # On Windows we must expand the patterns ourselves.
-  test_paths = [test for pattern in args.tests for test in glob.glob(pattern)]
-  for test in test_paths:
-argv = sys.argv[:]
-args = parser.parse_args()
-with open(test) as f:
-  input_lines = [l.rstrip() for l in f]
-
-first_line = input_lines[0] if input_lines else ""
-if 'autogenerated' in first_line and script_name not in first_line:
-  common.warn("Skipping test which wasn't autogenerated by " + script_name, test)
-  continue
-if first_line and 'autogenerated' in first_line:
-  args, argv = common.check_for_command(first_line, parser, args, argv)
-test_autogenerated_note = autogenerated_note + common.get_autogennote_suffix(parser, args)
-
-if args.update_only:
-  if not first_line or 'autogenerated' not in first_line:
-common.warn("Skipping test which isn't autogenerated: " + test)
-continue
-
-run_lines = common.find_run_lines(test, input_lines)
-
+  for ti in common.itertests(initial_args.tests, parser,
+ script_name='utils/' + script_name):
 # If requested we scrub trailing attribute annotations, e.g., '#0', together with whitespaces
-if args.scrub_attributes:
+if ti.args.scrub_attributes:
   common.SCRUB_TRAILING_WHITESPACE_TEST_RE = common.SCRUB_TRAILING_WHITESPACE_AND_ATTRIBUTES_RE
 else:
   common.SCRUB_TRAILING_WHITESPACE_TEST_RE = common.SCRUB_

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D78457#1991780 , @martong wrote:

> If a symbol is unused and garbage collected then that is not part of the path 
> constraint that leads to the ErrorNode, is it? So why should we care about 
> constraints on an unused symbol?


Unused means it is unused at a program point.
For example:

  void f(int sym1, int sym2) {
int sym3 = sym1 * sym2;
if (!sym1)
  return;
// Here sym1 is no longer used, it is garbage collected from the state.
if (sym3) {
  sym2 / 0; // This is a false positive. sym1 is 0 -> sym3 should be 0 as 
well, this branch is never taken. But to refute this path we need the info 
about sym1, that is not available in the ErrorNode state. It was garbage 
collected earlier.
} 
  }

I did not try this specific example. The constraint manager might be smart 
enough to not to report a false positive, but I hope it makes the idea clear 
why do we need the garbage collected symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> If a symbol is unused and garbage collected then that is not part of the path 
> constraint that leads to the ErrorNode, is it?

It is part of the path, it's just no longer accessible in the program, so it 
can't be observed at the //end// of the path. But it did play an important role 
at some point in the path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yup, just take a symbol that is completely unrelated to the bug report, make an 
impossible constraint on it that our normal constraint manager can't refute, 
forget about it. That'll be the example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for the test Shafik, that is pretty self explanatory!




Comment at: clang/lib/AST/ASTImporter.cpp:8161
   if (auto *ToRecord = dyn_cast(ToDC)) {
 auto *FromRecord = cast(FromDC);
 if (ToRecord->isCompleteDefinition()) {

What if we did this a bit differently? We could simply complete the From type 
if it is not completed, before getting into `ImportDefinition`.
```
if (ToRecord->isCompleteDefinition())
  return ToDC;
auto *FromRecord = cast(FromDC);
if (FromRecord->hasExternalLexicalStorage() &&
  !FromRecord->isCompleteDefinition())
FromRecord->getASTContext().getExternalSource()->CompleteType(
FromRecord);
```

This way we could get rid of the redundant calls of `ImportDefinition`. As we 
have now a call for the case when we don't have external storage and for the 
case when we have.


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

https://reviews.llvm.org/D78000



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

[PATCH] D78477: [profile] Don't crash when forking in several threads

[calixte via Phabricator via cfe-commits]

calixte updated this revision to Diff 258686.
calixte added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Export symbol for Darwin target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78477

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  compiler-rt/lib/profile/GCDAProfiling.c
  compiler-rt/test/profile/Inputs/instrprof-gcov-multithread_fork.cpp
  compiler-rt/test/profile/instrprof-gcov-multithread_fork.test
  llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp

Index: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -115,7 +115,8 @@
   // list.
   Function *
   insertCounterWriteout(ArrayRef>);
-  Function *insertFlush(ArrayRef>);
+  Function *insertReset(ArrayRef>);
+  Function *insertFlush(Function *ResetF);
 
   void AddFlushBeforeForkAndExec();
 
@@ -631,35 +632,79 @@
 }
 
 void GCOVProfiler::AddFlushBeforeForkAndExec() {
-  SmallVector ForkAndExecs;
+  SmallVector Forks;
+  SmallVector Execs;
   for (auto &F : M->functions()) {
 auto *TLI = &GetTLI(F);
 for (auto &I : instructions(F)) {
   if (CallInst *CI = dyn_cast(&I)) {
 if (Function *Callee = CI->getCalledFunction()) {
   LibFunc LF;
-  if (TLI->getLibFunc(*Callee, LF) &&
-  (LF == LibFunc_fork || LF == LibFunc_execl ||
-   LF == LibFunc_execle || LF == LibFunc_execlp ||
-   LF == LibFunc_execv || LF == LibFunc_execvp ||
-   LF == LibFunc_execve || LF == LibFunc_execvpe ||
-   LF == LibFunc_execvP)) {
-ForkAndExecs.push_back(&I);
+  if (TLI->getLibFunc(*Callee, LF)) {
+if (LF == LibFunc_fork) {
+#if !defined(_WIN32)
+  Forks.push_back(CI);
+#endif
+} else if (LF == LibFunc_execl || LF == LibFunc_execle ||
+   LF == LibFunc_execlp || LF == LibFunc_execv ||
+   LF == LibFunc_execvp || LF == LibFunc_execve ||
+   LF == LibFunc_execvpe || LF == LibFunc_execvP) {
+  Execs.push_back(CI);
+}
   }
 }
   }
 }
   }
 
-  // We need to split the block after the fork/exec call
-  // because else the counters for the lines after will be
-  // the same as before the call.
-  for (auto I : ForkAndExecs) {
-IRBuilder<> Builder(I);
+  for (auto F : Forks) {
+IRBuilder<> Builder(F);
+BasicBlock *Parent = F->getParent();
+auto NextInst = ++F->getIterator();
+
+// We've a fork so just reset the counters in the child process
+FunctionType *FTy = FunctionType::get(Builder.getInt32Ty(), {}, false);
+FunctionCallee GCOVFork = M->getOrInsertFunction("__gcov_fork", FTy);
+F->setCalledFunction(GCOVFork);
+if (NextInst != Parent->end()) {
+  // We split just after the fork to have a counter for the lines after
+  // Anyway there's a bug:
+  // void foo() { fork(); }
+  // void bar() { foo(); blah(); }
+  // then "blah();" will be called 2 times but showed as 1
+  // because "blah()" belongs to the same block as "foo();"
+  Parent->splitBasicBlock(NextInst);
+
+  // back() is a br instruction with a debug location
+  // equals to the one from NextAfterFork
+  // So to avoid to have two debug locs on two blocks just change it
+  DebugLoc Loc = F->getDebugLoc();
+  Parent->back().setDebugLoc(Loc);
+}
+  }
+
+  for (auto E : Execs) {
+IRBuilder<> Builder(E);
+BasicBlock *Parent = E->getParent();
+auto NextInst = ++E->getIterator();
+
+// Since the process is replaced by a new one we need to write out gcdas
+// No need to reset the counters since they'll be lost after the exec**
 FunctionType *FTy = FunctionType::get(Builder.getVoidTy(), {}, false);
-FunctionCallee GCOVFlush = M->getOrInsertFunction("__gcov_flush", FTy);
-Builder.CreateCall(GCOVFlush);
-I->getParent()->splitBasicBlock(I);
+FunctionCallee WriteoutF =
+M->getOrInsertFunction("llvm_writeout_files", FTy);
+Builder.CreateCall(WriteoutF);
+if (NextInst != Parent->end()) {
+  DebugLoc Loc = E->getDebugLoc();
+  Builder.SetInsertPoint(&*NextInst);
+  // If the exec** fails we must reset the counters since they've been
+  // dumped
+  FunctionCallee ResetF =
+  M->getOrInsertFunction("llvm_reset_counters", FTy);
+  Builder.CreateCall(ResetF)->setDebugLoc(Loc);
+  Parent->splitBasicBlock(NextInst);
+  Parent->back().setDebugLoc(Loc);
+}
   }
 }
 
@@ -851,7 +896,8 @@
 }
 
 Function *WriteoutF = insertCounterWriteout(CountersBySP);
-Function *FlushF = insertFlush(CountersBySP);
+Function *ResetF = insertReset(CountersBySP);
+Function *FlushF = insert

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

(sry for the noise, just keep thinking of new things to reply)

> To call the `visitor::finalizeVisitor` you would need almost the entire 
> world. (A ExprEngine, a CompilerInstance, etc.)

We already have unittests that mock these objects, cf. `ExprEngineConsumer` 
(feel free to extend as necessary).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78457



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

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

[Pavel Labath via Phabricator via cfe-commits]

labath added a comment.

David's example does work with gdb without -Wl,--gdb-index (the member variable 
is shown), presumably due to the aforementioned fuzzy matching. However, it 
does not work if gdb-index is enabled, nor with lldb (as lldb is always very 
index-oriented and assumes equality everywhere). That is definitely not ideal, 
though I'm not sure that means about this patch. This is definitely not the 
only difference in the formatting of DW_AT_names of templates. For example, 
`template operator<<(T, T)` will come out as `operator<< ` with 
gcc, but as `operator<<` with clang (with or without this patch).
OTOH, differences in type names are more likely to cause problems than is the 
case for functions/operators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76801



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

[clang] 1f67508 - [analyzer] Do not report CFError null dereference for nonnull params.

[Artem Dergachev via cfe-commits]

Author: Valeriy Savchenko
Date: 2020-04-20T12:33:01+03:00
New Revision: 1f67508b7fedb3a0c8ea55a1f5674346563e2bf2

URL: 
https://github.com/llvm/llvm-project/commit/1f67508b7fedb3a0c8ea55a1f5674346563e2bf2
DIFF: 
https://github.com/llvm/llvm-project/commit/1f67508b7fedb3a0c8ea55a1f5674346563e2bf2.diff

LOG: [analyzer] Do not report CFError null dereference for nonnull params.

We want to trust user type annotations and stop assuming pointers declared
as nonnull still can be null. This functionality is implemented as part
of NonNullParamChecker because it already checks parameter attributes.
Whenever we start analyzing a new function, we assume that all parameters
with 'nonnull' attribute are indeed non-null.

Patch by Valeriy Savchenko!

Differential Revision: https://reviews.llvm.org/D77806

Added: 
clang/test/Analysis/UserNullabilityAnnotations.m
clang/test/Analysis/nonnull.cpp

Modified: 
clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
clang/test/Analysis/CheckNSError.m

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
index 1bff681fb0d0..c3c6a69a222c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -14,8 +14,9 @@
 //
 
//===--===//
 
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/Attr.h"
+#include "clang/Analysis/AnyCall.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -28,44 +29,82 @@ using namespace ento;
 
 namespace {
 class NonNullParamChecker
-  : public Checker< check::PreCall, EventDispatcher > {
+: public Checker> {
   mutable std::unique_ptr BTAttrNonNull;
   mutable std::unique_ptr BTNullRefArg;
 
 public:
-
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkBeginFunction(CheckerContext &C) const;
 
   std::unique_ptr
-  genReportNullAttrNonNull(const ExplodedNode *ErrorN,
-   const Expr *ArgE,
+  genReportNullAttrNonNull(const ExplodedNode *ErrorN, const Expr *ArgE,
unsigned IdxOfArg) const;
   std::unique_ptr
   genReportReferenceToNullPointer(const ExplodedNode *ErrorN,
   const Expr *ArgE) const;
 };
-} // end anonymous namespace
 
-/// \return Bitvector marking non-null attributes.
-static llvm::SmallBitVector getNonNullAttrs(const CallEvent &Call) {
+template 
+void setBitsAccordingToFunctionAttributes(const CallType &Call,
+  llvm::SmallBitVector &AttrNonNull) {
   const Decl *FD = Call.getDecl();
-  unsigned NumArgs = Call.getNumArgs();
-  llvm::SmallBitVector AttrNonNull(NumArgs);
+
   for (const auto *NonNull : FD->specific_attrs()) {
 if (!NonNull->args_size()) {
-  AttrNonNull.set(0, NumArgs);
+  // Lack of attribute parameters means that all of the parameters are
+  // implicitly marked as non-null.
+  AttrNonNull.set();
   break;
 }
+
 for (const ParamIdx &Idx : NonNull->args()) {
+  // 'nonnull' attribute's parameters are 1-based and should be adjusted to
+  // match actual AST parameter/argument indices.
   unsigned IdxAST = Idx.getASTIndex();
-  if (IdxAST >= NumArgs)
+  if (IdxAST >= AttrNonNull.size())
 continue;
   AttrNonNull.set(IdxAST);
 }
   }
+}
+
+template 
+void setBitsAccordingToParameterAttributes(const CallType &Call,
+   llvm::SmallBitVector &AttrNonNull) {
+  for (const ParmVarDecl *Parameter : Call.parameters()) {
+unsigned ParameterIndex = Parameter->getFunctionScopeIndex();
+if (ParameterIndex == AttrNonNull.size())
+  break;
+
+if (Parameter->hasAttr())
+  AttrNonNull.set(ParameterIndex);
+  }
+}
+
+template 
+llvm::SmallBitVector getNonNullAttrsImpl(const CallType &Call,
+ unsigned ExpectedSize) {
+  llvm::SmallBitVector AttrNonNull(ExpectedSize);
+
+  setBitsAccordingToFunctionAttributes(Call, AttrNonNull);
+  setBitsAccordingToParameterAttributes(Call, AttrNonNull);
+
   return AttrNonNull;
 }
 
+/// \return Bitvector marking non-null attributes.
+llvm::SmallBitVector getNonNullAttrs(const CallEvent &Call) {
+  return getNonNullAttrsImpl(Call, Call.getNumArgs());
+}
+
+/// \return Bitvector marking non-null attributes.
+llvm::SmallBitVector getNonNullAttrs(const AnyCall &Call) {
+  return getNonNullAttrsImpl(Call, Call.param_size());
+}
+} // end anonymous namespace
+
 void NonNullParamChecker::checkPreCall(const CallEvent &Call,

[clang] 09a1f09 - [analyzer] Do not report NSError null dereference for _Nonnull params.

[Artem Dergachev via cfe-commits]

Author: Valeriy Savchenko
Date: 2020-04-20T12:33:01+03:00
New Revision: 09a1f090509eb5971ade7089330008c6e4024d30

URL: 
https://github.com/llvm/llvm-project/commit/09a1f090509eb5971ade7089330008c6e4024d30
DIFF: 
https://github.com/llvm/llvm-project/commit/09a1f090509eb5971ade7089330008c6e4024d30.diff

LOG: [analyzer] Do not report NSError null dereference for _Nonnull params.

We want to trust user type annotations and stop assuming pointers declared
as _Nonnull still can be null. This functionality is implemented as part
of NullabilityChecker as it already tracks non-null types.

Patch by Valeriy Savchenko!

Differential Revision: https://reviews.llvm.org/D77722

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
clang/test/Analysis/CheckNSError.m

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 0b8d100992a2..565d0eeef704 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -81,7 +81,7 @@ class NullabilityChecker
 : public Checker,
  check::PostCall, check::PostStmt,
  check::PostObjCMessage, check::DeadSymbols,
- check::Event> {
+ check::Location, check::Event> {
   mutable std::unique_ptr BT;
 
 public:
@@ -101,6 +101,8 @@ class NullabilityChecker
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   void checkEvent(ImplicitNullDerefEvent Event) const;
+  void checkLocation(SVal Location, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
   const char *Sep) const override;
@@ -503,6 +505,52 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent 
Event) const {
   }
 }
 
+// Whenever we see a load from a typed memory region that's been annotated as
+// 'nonnull', we want to trust the user on that and assume that it is is indeed
+// non-null.
+//
+// We do so even if the value is known to have been assigned to null.
+// The user should be warned on assigning the null value to a non-null pointer
+// as opposed to warning on the later dereference of this pointer.
+//
+// \code
+//   int * _Nonnull var = 0; // we want to warn the user here...
+//   // . . .
+//   *var = 42;  // ...and not here
+// \endcode
+void NullabilityChecker::checkLocation(SVal Location, bool IsLoad,
+   const Stmt *S,
+   CheckerContext &Context) const {
+  // We should care only about loads.
+  // The main idea is to add a constraint whenever we're loading a value from
+  // an annotated pointer type.
+  if (!IsLoad)
+return;
+
+  // Annotations that we want to consider make sense only for types.
+  const auto *Region =
+  dyn_cast_or_null(Location.getAsRegion());
+  if (!Region)
+return;
+
+  ProgramStateRef State = Context.getState();
+
+  auto StoredVal = State->getSVal(Region).getAs();
+  if (!StoredVal)
+return;
+
+  Nullability NullabilityOfTheLoadedValue =
+  getNullabilityAnnotation(Region->getValueType());
+
+  if (NullabilityOfTheLoadedValue == Nullability::Nonnull) {
+// It doesn't matter what we think about this particular pointer, it should
+// be considered non-null as annotated by the developer.
+if (ProgramStateRef NewState = State->assume(*StoredVal, true)) {
+  Context.addTransition(NewState);
+}
+  }
+}
+
 /// Find the outermost subexpression of E that is not an implicit cast.
 /// This looks through the implicit casts to _Nonnull that ARC adds to
 /// return expressions of ObjC types when the return type of the function or

diff  --git a/clang/test/Analysis/CheckNSError.m 
b/clang/test/Analysis/CheckNSError.m
index 6de98e85efe3..10890a8627a1 100644
--- a/clang/test/Analysis/CheckNSError.m
+++ b/clang/test/Analysis/CheckNSError.m
@@ -1,5 +1,8 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,osx.cocoa.NSError,osx.coreFoundation.CFError 
-analyzer-store=region -verify -Wno-objc-root-class %s
-
+// RUN: %clang_analyze_cc1 -verify -Wno-objc-root-class %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability \
+// RUN:   -analyzer-checker=osx.cocoa.NSError \
+// RUN:   -analyzer-checker=osx.coreFoundation.CFError
 
 typedef signed char BOOL;
 typedef int NSInteger;
@@ -18,6 +21,7 @@ + (id)errorWithDomain:(NSString *)domain code:(NSInteger)code 
userInfo:(NSDictio
 @interface A
 - (void)myMethodWhichMayFail:(NSError **)error;
 - (BOOL)myMethodWhichMayFail2:(NSError **)error;
+- (BOOL)myMethodWhichMayFail3:(NSError **_Nonnull)error;
 @end
 
 @implementation A
@@ -29,6 +33,

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:125
+Msg = std::string(llvm::formatv(
+"Possibly not enough {0} bytes for array allocation which "
+"requires "

f00kat wrote:
> martong wrote:
> > Maybe the below could be a wording that's more easy to follow?
> > `{0} bytes is possibly not enough for array allocation which ...`
> Yeah. Sure. I have some troubles with english :)
Why do we say "possibly"? Where does the uncertainty come from?



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189
+  if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+if (const ElementRegion *TheElementRegion =
+MRegion->getAs()) {

martong wrote:
> f00kat wrote:
> > f00kat wrote:
> > > NoQ wrote:
> > > > NoQ wrote:
> > > > > The sequence of `FieldRegion`s and `ElementRegion`s on top of a base 
> > > > > region may be arbitrary: `var.a[0].b[1][2].c.d[3]` etc.
> > > > > 
> > > > > I'd rather unwrap those regions one-by-one in a loop and look at the 
> > > > > alignment of each layer.
> > > > Alternatively, just decompose the whole region into base region and 
> > > > offset and see if base region has the necessary alignment and the 
> > > > offset is divisible by the necessary alignment.
> > > > The sequence of FieldRegions and ElementRegions on top of a base region 
> > > > may be arbitrary: var.a[0].b[1][2].c.d[3] etc.
> > > 
> > > But i think(hope) I already do this and even have tests for this cases. 
> > > For example
> > > ```void test22() {
> > >   struct alignas(alignof(short)) Z {
> > > char p;
> > > char c[10];
> > >   };
> > > 
> > >   struct Y {
> > > char p;
> > > Z b[10];
> > >   };
> > > 
> > >   struct X {
> > > Y a[10];
> > >   } Xi; // expected-note {{'Xi' initialized here}}
> > > 
> > >   // ok. 2(X align) + 1 (offset Y.p) + 1(align Z.p to 'short') + 1(offset 
> > > Z.p) + 3(index)
> > >   ::new (&Xi.a[0].b[0].c[3]) long;
> > > }```
> > > 
> > > Cases with multidimensional arrays will also be handled correctly because 
> > > method 'TheElementRegion->getAsArrayOffset()' calculates the offset for 
> > > multidimensional arrays
> > > ```void testXX() {
> > >   struct Y {
> > >   char p;
> > >   char b[10][10];
> > >   };
> > > 
> > >   struct X {
> > >   Y a[10];
> > >   } Xi;
> > > 
> > >   ::new (&Xi.a[0].b[0][0]) long;
> > > }```
> > > 
> > > I can explain the code below for ElementRegion if needed.
> > ?
> Yeah, the tests are convincing and I think that you are handling the regions 
> well.
> 
> On the other hand, this code is getting really complex, we should make it 
> easier to read and understand.
> E.g. `FieldOffsetValue` should be explained more, is it the offset started 
> from the the start address of the multidimensional array, or it is just the 
> offset from one element's start address?
> Also, you have two variables named as `Offset`. They are offsets from which 
> starting address? Perhaps we should have in the comments a running example, 
> maybe for `&Xi.a[0].b[0][0]`? I mean is `FieldOffseValue` is standing for `b` 
> or for `a`?
> Alternatively, just decompose the whole region into base region and offset 
> and see if base region has the necessary alignment and the offset is 
> divisible by the necessary alignment.

I expect this to be, like, 5 lines of code. I don't understand why the current 
code is so complicated, it looks like you're considering multiple cases but 
ultimately doing the same thing.


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

https://reviews.llvm.org/D76996



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

[PATCH] D78024: [FileCheck] - Fix the false positive when -implicit-check-not is used with an unknown -check-prefix.

[George Rimar via Phabricator via cfe-commits]

grimar added a comment.

In D78024#1989287 , @probinson wrote:

> I'm just reading this review for the first time, and my thought was, why is 
> this interacting with implicit-check-not?


It was interacting (this patch was committed and fixed the issue), because the 
code used the logic to add an EOF pattern
for "any trailing --implicit-check-not/CHECK-DAG/-NOTs" before the error about 
"no check strings found with prefix" was reported.
I.e. this logic suppressed the error, because `CheckStrings` was not empty 
anymore:

if (CheckStrings->empty()) {
  errs() << "error: no check strings found with prefix"
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78024



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

[PATCH] D77722: [analyzer] Do not report NSError null dereference for _Nonnull params

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG09a1f090509e: [analyzer] Do not report NSError null 
dereference for _Nonnull params. (authored by vsavchenko, committed by 
dergachev.a).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77722

Files:
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/test/Analysis/CheckNSError.m

Index: clang/test/Analysis/CheckNSError.m
===
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -1,5 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.cocoa.NSError,osx.coreFoundation.CFError -analyzer-store=region -verify -Wno-objc-root-class %s
-
+// RUN: %clang_analyze_cc1 -verify -Wno-objc-root-class %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability \
+// RUN:   -analyzer-checker=osx.cocoa.NSError \
+// RUN:   -analyzer-checker=osx.coreFoundation.CFError
 
 typedef signed char BOOL;
 typedef int NSInteger;
@@ -18,6 +21,7 @@
 @interface A
 - (void)myMethodWhichMayFail:(NSError **)error;
 - (BOOL)myMethodWhichMayFail2:(NSError **)error;
+- (BOOL)myMethodWhichMayFail3:(NSError **_Nonnull)error;
 @end
 
 @implementation A
@@ -29,6 +33,11 @@
   if (error) *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // no-warning
   return 0;
 }
+
+- (BOOL)myMethodWhichMayFail3:(NSError **_Nonnull)error { // no-warning
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // no-warning
+  return 0;
+}
 @end
 
 struct __CFError {};
Index: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -81,7 +81,7 @@
 : public Checker,
  check::PostCall, check::PostStmt,
  check::PostObjCMessage, check::DeadSymbols,
- check::Event> {
+ check::Location, check::Event> {
   mutable std::unique_ptr BT;
 
 public:
@@ -101,6 +101,8 @@
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   void checkEvent(ImplicitNullDerefEvent Event) const;
+  void checkLocation(SVal Location, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
   const char *Sep) const override;
@@ -503,6 +505,52 @@
   }
 }
 
+// Whenever we see a load from a typed memory region that's been annotated as
+// 'nonnull', we want to trust the user on that and assume that it is is indeed
+// non-null.
+//
+// We do so even if the value is known to have been assigned to null.
+// The user should be warned on assigning the null value to a non-null pointer
+// as opposed to warning on the later dereference of this pointer.
+//
+// \code
+//   int * _Nonnull var = 0; // we want to warn the user here...
+//   // . . .
+//   *var = 42;  // ...and not here
+// \endcode
+void NullabilityChecker::checkLocation(SVal Location, bool IsLoad,
+   const Stmt *S,
+   CheckerContext &Context) const {
+  // We should care only about loads.
+  // The main idea is to add a constraint whenever we're loading a value from
+  // an annotated pointer type.
+  if (!IsLoad)
+return;
+
+  // Annotations that we want to consider make sense only for types.
+  const auto *Region =
+  dyn_cast_or_null(Location.getAsRegion());
+  if (!Region)
+return;
+
+  ProgramStateRef State = Context.getState();
+
+  auto StoredVal = State->getSVal(Region).getAs();
+  if (!StoredVal)
+return;
+
+  Nullability NullabilityOfTheLoadedValue =
+  getNullabilityAnnotation(Region->getValueType());
+
+  if (NullabilityOfTheLoadedValue == Nullability::Nonnull) {
+// It doesn't matter what we think about this particular pointer, it should
+// be considered non-null as annotated by the developer.
+if (ProgramStateRef NewState = State->assume(*StoredVal, true)) {
+  Context.addTransition(NewState);
+}
+  }
+}
+
 /// Find the outermost subexpression of E that is not an implicit cast.
 /// This looks through the implicit casts to _Nonnull that ARC adds to
 /// return expressions of ObjC types when the return type of the function or
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:113
+
+bool isObjCClassType(QualType Type) {
+  if (const auto *PointerType = dyn_cast(Type)) {

`static`?



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:134
+  // 1. Class is written directly in the message:
+  // \code
+  //   [ActualClass classMethod];

These comments will never be parsed by doxygen, i don't think those `\code` 
thingies are needed here. Or is it some editor/IDE-specific thingies that i'm 
not educated about?



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:193
+// Types in Class objects can be ONLY Objective-C types
+return {cast(DTI.getType())};
+  }

Why are we discarding `CanBeSubClassed` here?



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

I believe this is pretty much always the case. At least whenever 
`getInstanceReceiver()` is available. Another exception seem to be when 
`ReceiverSVal` is an `UnknownVal` (in this case `self` is going to be 
`SymbolRegionValue` because it's never set in the Store), but that's it. I 
inferred this by looking at `ObjCMethodCall::getInitialStackFrameContents()`.

I think we should have used `getSelfSVal()` to begin with.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:919
+
+  ReceiverRuntimeType.Type->getSuperClassType();
+  QualType ReceiverClassType(ReceiverRuntimeType.Type, 0);

I don't think this line of code does anything.

(time for some `LLVM_NODISCARD`???)



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:1219-1226
+  // NOTE: This cache is essentially a "global" variable, but it
+  // only gets lazily created when we get here.  The value of the
+  // cache probably comes from it being global across ExprEngines,
+  // where the same queries may get issued.  If we are worried about
+  // concurrency, or possibly loading/unloading ASTs, etc., we may
+  // need to revisit this someday.  In terms of memory, this table
+  // stays around until clang quits, which also may be bad if we

Before i forget: Stuff that's shared across analyses usually lives in 
`AnalysisConsumer`. Cf. `FunctionSummaries`. This is the intended way of 
avoiding globals in such cases.



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:1293
+  if (Receiver == getSelfSVal().getAsRegion()) {
 return RuntimeDefinition(findDefiningRedecl(E->getMethodDecl()));
+  }

Wait, how do we get a decl here that's anyhow relevant if the compiler doesn't 
even know that it's a class method?



Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:85-87
+  if (const DynamicTypeInfo *DTI = State->get(Sym))
+return *DTI;
+  return {};

My favorite idiom for this stuff so far:
```lang=c++
  const DynamicTypeInfo *DTI = State->get(Sym);
  return DTI ? *DTI : {};
```
(not sure `?:` will typecheck correctly in case of `{}` though)



Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:147
+static bool isLive(SymbolReaper &SR, const MemRegion *MR) {
+  return SR.isLiveRegion(MR);
+}

Feel free to rename `isLiveRegion` instead if it helps :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D75068: libclang: Add static build support for Windows

[Cristian Adam via Phabricator via cfe-commits]

cristian.adam added a comment.

@thakis  ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75068



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

[PATCH] D78204: [AArch64][SVE] Remove LD1/ST1 dependency on llvm.masked.load/store

[Kerry McLaughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG33ffce5414ec: [AArch64][SVE] Remove LD1/ST1 dependency on 
llvm.masked.load/store (authored by kmclaughlin).

Changed prior to commit:
  https://reviews.llvm.org/D78204?vs=257702&id=258694#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78204

Files:
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-ld1.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-ldst1.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
@@ -0,0 +1,367 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; ST1B
+;
+
+define void @st1b_i8( %data,  %pred, i8* %addr) {
+; CHECK-LABEL: st1b_i8:
+; CHECK: st1b { z0.b }, p0, [x0]
+; CHECK-NEXT: ret
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,
+   %pred,
+  i8* %addr)
+  ret void
+}
+
+define void @st1b_h( %data,  %pred, i8* %addr) {
+; CHECK-LABEL: st1b_h:
+; CHECK: st1b { z0.h }, p0, [x0]
+; CHECK-NEXT: ret
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv8i8( %trunc,
+  %pred,
+ i8* %addr)
+  ret void
+}
+
+define void @st1b_s( %data,  %pred, i8* %addr) {
+; CHECK-LABEL: st1b_s:
+; CHECK: st1b { z0.s }, p0, [x0]
+; CHECK-NEXT: ret
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv4i8( %trunc,
+  %pred,
+ i8* %addr)
+  ret void
+}
+
+define void @st1b_d( %data,  %pred, i8* %addr) {
+; CHECK-LABEL: st1b_d:
+; CHECK: st1b { z0.d }, p0, [x0]
+; CHECK-NEXT: ret
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv2i8( %trunc,
+  %pred,
+ i8* %addr)
+  ret void
+}
+
+define void @st1b_upper_bound( %data,  %pg, i8* %a) {
+; CHECK-LABEL: st1b_upper_bound:
+; CHECK: st1b { z0.b }, p0, [x0, #7, mul vl]
+; CHECK-NEXT: ret
+  %base_scalable = bitcast i8* %a to *
+  %base = getelementptr , * %base_scalable, i64 7
+  %base_scalar = bitcast * %base to i8*
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,  %pg, i8* %base_scalar)
+  ret void
+}
+
+define void @st1b_inbound( %data,  %pg, i8* %a) {
+; CHECK-LABEL: st1b_inbound:
+; CHECK: st1b { z0.b }, p0, [x0, #1, mul vl]
+; CHECK-NEXT: ret
+  %base_scalable = bitcast i8* %a to *
+  %base = getelementptr , * %base_scalable, i64 1
+  %base_scalar = bitcast * %base to i8*
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,  %pg, i8* %base_scalar)
+  ret void
+}
+
+define void @st1b_lower_bound( %data,  %pg, i8* %a) {
+; CHECK-LABEL: st1b_lower_bound:
+; CHECK: st1b { z0.b }, p0, [x0, #-8, mul vl]
+; CHECK-NEXT: ret
+  %base_scalable = bitcast i8* %a to *
+  %base = getelementptr , * %base_scalable, i64 -8
+  %base_scalar = bitcast * %base to i8*
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,  %pg, i8* %base_scalar)
+  ret void
+}
+
+define void @st1b_out_of_upper_bound( %data,  %pg, i8* %a) {
+; CHECK-LABEL: st1b_out_of_upper_bound:
+; CHECK: rdvl x[[OFFSET:[0-9]+]], #8
+; CHECK: add x[[BASE:[0-9]+]], x0, x[[OFFSET]]
+; CHECK: st1b { z0.b }, p0, [x[[BASE]]]
+; CHECK-NEXT: ret
+  %base_scalable = bitcast i8* %a to *
+  %base = getelementptr , * %base_scalable, i64 8
+  %base_scalar = bitcast * %base to i8*
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,  %pg, i8* %base_scalar)
+  ret void
+}
+
+define void @st1b_out_of_lower_bound( %data,  %pg, i8* %a) {
+; CHECK-LABEL: st1b_out_of_lower_bound:
+; CHECK: rdvl x[[OFFSET:[0-9]+]], #-9
+; CHECK: add x[[BASE:[0-9]+]], x0, x[[OFFSET]]
+; CHECK: st1b { z0.b }, p0, [x[[BASE]]]
+; CHECK-NEXT: ret
+  %base_scalable = bitcast i8* %a to *
+  %base = getelementptr , * %base_scalable, i64 -9
+  %base_scalar = bitcast * %base to i8*
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,  %pg, i8* %base_scalar)
+  ret void
+}
+
+define void @st1b_s_inbound( %data,  %pg, i8* %a) {
+; CHECK-LABEL: st1b_s_inbound:
+; CHECK: st1b { z0.s }, p0, [x0, #7, mul vl]
+; CHECK-NEXT: ret
+  %base_scalable = bitcast i8* %a to *
+  %base = getelementptr , * %base_scalable, i64 7
+  %base_scalar = bitcast * %base to i8*
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv4i8( %trunc,  %pg, i8* %base_scalar)
+  ret void
+}
+
+define void @st1b_h_inbound( %data,  %pg, i8* %a) {
+; CHECK-LABEL: st1b_h_inbound:
+; CHECK: st1b { z0.h }, p0, [x0, #1, mul vl]
+; CHECK-NEXT: ret
+  %base_

[PATCH] D77806: [analyzer] Do not report CFError null dereference for nonnull params

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f67508b7fed: [analyzer] Do not report CFError null 
dereference for nonnull params. (authored by vsavchenko, committed by 
dergachev.a).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77806

Files:
  clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  clang/test/Analysis/CheckNSError.m
  clang/test/Analysis/UserNullabilityAnnotations.m
  clang/test/Analysis/nonnull.cpp

Index: clang/test/Analysis/nonnull.cpp
===
--- /dev/null
+++ clang/test/Analysis/nonnull.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core -verify %s
+
+void nonnull [[gnu::nonnull]] (int *q);
+
+void f1(int *p) {
+  if (p)
+return;
+  nonnull(p); //expected-warning{{nonnull}}
+}
+
+void f2(int *p) {
+  if (p)
+return;
+  auto lambda = [](int *q) __attribute__((nonnull)){};
+  lambda(p); //expected-warning{{nonnull}}
+}
+
+template 
+void variadicNonnull(ARGS... args) __attribute__((nonnull));
+
+void f3(int a, float b, int *p) {
+  if (p)
+return;
+  variadicNonnull(a, b, p); //expected-warning{{nonnull}}
+}
+
+int globalVar = 15;
+void moreParamsThanArgs [[gnu::nonnull(2, 4)]] (int a, int *p, int b = 42, int *q = &globalVar);
+
+void f4(int a, int *p) {
+  if (p)
+return;
+  moreParamsThanArgs(a, p); //expected-warning{{nonnull}}
+}
Index: clang/test/Analysis/UserNullabilityAnnotations.m
===
--- /dev/null
+++ clang/test/Analysis/UserNullabilityAnnotations.m
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -verify -Wno-objc-root-class %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+void clang_analyzer_eval(int);
+
+@interface TestFunctionLevelAnnotations
+- (void)method1:(int *_Nonnull)x;
+- (void)method2:(int *)x __attribute__((nonnull));
+@end
+
+@implementation TestFunctionLevelAnnotations
+- (void)method1:(int *_Nonnull)x {
+  clang_analyzer_eval(x != 0); // expected-warning{{TRUE}}
+}
+
+- (void)method2:(int *)x {
+  clang_analyzer_eval(x != 0); // expected-warning{{TRUE}}
+}
+@end
+
+typedef struct NestedNonnullMember {
+  struct NestedNonnullMember *Child;
+  int *_Nonnull Value;
+} NestedNonnullMember;
+
+NestedNonnullMember *foo();
+
+void f1(NestedNonnullMember *Root) {
+  NestedNonnullMember *Grandson = Root->Child->Child;
+
+  clang_analyzer_eval(Root->Value != 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(Grandson->Value != 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(foo()->Child->Value != 0); // expected-warning{{TRUE}}
+}
Index: clang/test/Analysis/CheckNSError.m
===
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -22,6 +22,7 @@
 - (void)myMethodWhichMayFail:(NSError **)error;
 - (BOOL)myMethodWhichMayFail2:(NSError **)error;
 - (BOOL)myMethodWhichMayFail3:(NSError **_Nonnull)error;
+- (BOOL)myMethodWhichMayFail4:(NSError **)error __attribute__((nonnull));
 @end
 
 @implementation A
@@ -38,6 +39,11 @@
   *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // no-warning
   return 0;
 }
+
+- (BOOL)myMethodWhichMayFail4:(NSError **)error { // no-warning
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // no-warning
+  return 0;
+}
 @end
 
 struct __CFError {};
@@ -62,4 +68,17 @@
   return 0;
 }
 
+int __attribute__((nonnull)) f4(CFErrorRef *error) {
+  *error = 0; // no-warning
+  return 0;
+}
 
+int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
+  *error = 0; // expected-warning {{Potential null dereference}}
+  return 0;
+}
+
+int __attribute__((nonnull(2))) f6(int *x, CFErrorRef *error) {
+  *error = 0; // no-warning
+  return 0;
+}
Index: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -14,8 +14,9 @@
 //
 //===--===//
 
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/Attr.h"
+#include "clang/Analysis/AnyCall.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -28,44 +29,82 @@
 
 namespace {
 class NonNullParamChecker
-  : public Checker< check::PreCall, EventDispatcher > {
+: public Checker> {
   mutable std::unique_ptr BTAttrNonNull;
   mutable std::unique_ptr BTNullRefArg;
 
 public:
-
   void che

[PATCH] D78454: [clangd] Highlight related control flow.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:628
+return FD->getBody();
+  if (const auto *FD = N.get())
+return FD->getBody();

You are checking for FunctionDecl twice



Comment at: clang-tools-extra/clangd/XRefs.cpp:654
+  // Types of control-flow statements we might highlight.
+  enum Target {
+Break = 1,

What about "goto"? It's more difficult to figure out if it breaks out of the 
loop or just jumps inside it, but it might still be worth highlighting it, even 
unconditionally (inside a loop only, not function).



Comment at: clang-tools-extra/clangd/XRefs.cpp:761
+  auto CaseBefore = std::prev(CaseAfter);
+  // ... rewind CaseBefore to the first in a `case A: case B: ...` sequence.
+  while (CaseBefore != Cases.begin() &&

That's a nice feature, but there's no test for it. Can you add it?



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:119
 
+TEST(HighlightsTest, ControlFlow) {
+  const char *Tests[] = {

Can you add a test for macros? I think highlighting like ASSIGN_OR_RETURN(int 
x, foo()); would be great, but even if we don't support that, just having a 
test that we don't crash would be valuable, IMHO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78454



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

[PATCH] D78481: [ARM] Release notes for the Custom Datapath Extension (CDE)

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki created this revision.
miyuki added reviewers: kristof.beyls, simon_tatham.
Herald added subscribers: cfe-commits, danielkiss.
Herald added a project: clang.

This change mentions CDE assembly in the LLVM release notes and CDE
intrinsics in both Clang and LLVM release notes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78481

Files:
  clang/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -76,6 +76,11 @@
   set.  now supports the complete API defined in the Arm C
   Language Extensions.
 
+* Added support for assembly for the optional Custom Datapath Extension (CDE)
+  for Arm M-profile targets.
+
+* Implemented C-language intrinsics  for the CDE instruction set.
+
 Changes to the MIPS Target
 --
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -61,6 +61,8 @@
   v8.1-M MVE instruction set.  supports the complete API defined
   in the Arm C Language Extensions.
 
+- For the ARM target, C-language intrinsics  for the CDE instruction
+  set are now provided.
 
 * clang adds support for a set of  extended integer types (``_ExtInt(N)``) that
   permit non-power of 2 integers, exposing the LLVM integer types. Since a 
major


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -76,6 +76,11 @@
   set.  now supports the complete API defined in the Arm C
   Language Extensions.
 
+* Added support for assembly for the optional Custom Datapath Extension (CDE)
+  for Arm M-profile targets.
+
+* Implemented C-language intrinsics  for the CDE instruction set.
+
 Changes to the MIPS Target
 --
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -61,6 +61,8 @@
   v8.1-M MVE instruction set.  supports the complete API defined
   in the Arm C Language Extensions.
 
+- For the ARM target, C-language intrinsics  for the CDE instruction
+  set are now provided.
 
 * clang adds support for a set of  extended integer types (``_ExtInt(N)``) that
   permit non-power of 2 integers, exposing the LLVM integer types. Since a major
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko marked 7 inline comments as done.
vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:113
+
+bool isObjCClassType(QualType Type) {
+  if (const auto *PointerType = dyn_cast(Type)) {

NoQ wrote:
> `static`?
It is all inside of `anonymous` namespace, so no need in putting static here.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:134
+  // 1. Class is written directly in the message:
+  // \code
+  //   [ActualClass classMethod];

NoQ wrote:
> These comments will never be parsed by doxygen, i don't think those `\code` 
> thingies are needed here. Or is it some editor/IDE-specific thingies that i'm 
> not educated about?
No, you are correct. It's more like a matter of habit and consistent look of 
the comment.  I would anyway prefer to somehow tell the reader that this is a 
snippet. It can be with a more or less familiar way of doing it.  (I've seen a 
good amount of doxygen-style comments for `static` functions in the project, I 
guess similar logic applies to those as well)



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:193
+// Types in Class objects can be ONLY Objective-C types
+return {cast(DTI.getType())};
+  }

NoQ wrote:
> Why are we discarding `CanBeSubClassed` here?
Good point, we should propagate it further



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

NoQ wrote:
> I believe this is pretty much always the case. At least whenever 
> `getInstanceReceiver()` is available. Another exception seem to be when 
> `ReceiverSVal` is an `UnknownVal` (in this case `self` is going to be 
> `SymbolRegionValue` because it's never set in the Store), but that's it. I 
> inferred this by looking at `ObjCMethodCall::getInitialStackFrameContents()`.
> 
> I think we should have used `getSelfSVal()` to begin with.
> I believe this is pretty much always the case.

I didn't quite get what you mean by that





Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:919
+
+  ReceiverRuntimeType.Type->getSuperClassType();
+  QualType ReceiverClassType(ReceiverRuntimeType.Type, 0);

NoQ wrote:
> I don't think this line of code does anything.
> 
> (time for some `LLVM_NODISCARD`???)
Whoops, that's true!



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:1219-1226
+  // NOTE: This cache is essentially a "global" variable, but it
+  // only gets lazily created when we get here.  The value of the
+  // cache probably comes from it being global across ExprEngines,
+  // where the same queries may get issued.  If we are worried about
+  // concurrency, or possibly loading/unloading ASTs, etc., we may
+  // need to revisit this someday.  In terms of memory, this table
+  // stays around until clang quits, which also may be bad if we

NoQ wrote:
> Before i forget: Stuff that's shared across analyses usually lives in 
> `AnalysisConsumer`. Cf. `FunctionSummaries`. This is the intended way of 
> avoiding globals in such cases.
We can re-do it in a separate commit I guess. 



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:1293
+  if (Receiver == getSelfSVal().getAsRegion()) {
 return RuntimeDefinition(findDefiningRedecl(E->getMethodDecl()));
+  }

NoQ wrote:
> Wait, how do we get a decl here that's anyhow relevant if the compiler 
> doesn't even know that it's a class method?
Compiler knows it, but still marks it as an instance method (because 
technically `self` is an object).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

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

Thanks Endre for the docs! I checked the CI job also, seemed okay, so, I think 
we are ready! Nice work! Let's do the commit!




Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:26
+The index, which maps function USR names to PCH dumps containing them must 
also be generated by the
+`clang-extdef-mapping` clang tool. The external mapping creation implicitly 
uses a compilation command database to
+determine the compilation flags used.

Maybe just simply write `This tool uses a compilation command database ...`
(Same below with the on-demand version.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665



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

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

vsavchenko wrote:
> NoQ wrote:
> > I believe this is pretty much always the case. At least whenever 
> > `getInstanceReceiver()` is available. Another exception seem to be when 
> > `ReceiverSVal` is an `UnknownVal` (in this case `self` is going to be 
> > `SymbolRegionValue` because it's never set in the Store), but that's it. I 
> > inferred this by looking at 
> > `ObjCMethodCall::getInitialStackFrameContents()`.
> > 
> > I think we should have used `getSelfSVal()` to begin with.
> > I believe this is pretty much always the case.
> 
> I didn't quite get what you mean by that
> 
> 
What i'm trying to say is that `C.getSVal(RecE)` and `Message.getSelfSVal()` 
and `Message.getReceiverSVal()` are basically the same `SVal`. It shouldn't be 
necessary to check both or check whether they're the same; you must have meant 
to check for something else, probably something purely syntactic.



> I inferred this by looking at ObjCMethodCall::getInitialStackFrameContents().

Wait, so it's only true for inlined methods. For non-inlined methods 
`getSelfSVal()` will be unknown :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D77221: [AVR] Rework MCU family detection to support more AVR MCUs

[Ayke via Phabricator via cfe-commits]

aykevl added inline comments.



Comment at: clang/lib/Basic/Targets/AVR.cpp:35
+auto FamilyInfo = llvm::AVR::getFamilyInfo(Family.getValue());
+Builder.defineMacro("__AVR_ARCH__", Twine(FamilyInfo.Number));
+  }

This should have tests. Take a look at D78117 for how you can add them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77221



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:56
+  load_threshold_reached,
+  ambiguous_compile_commands_database
 };

The two enum values refer to compilation database and compile command database. 
I'd prefer to use the same wording in both values.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:227
+/// Identifier.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

I am not sure if this is good design.
Here, if the meaning of the `Identifier` depends on the subclass, the caller of 
this method always needs to be aware of the dynamic type of the object. What is 
the point of a common base class if we always need to know the dynamic type?

Looking at the code this does not look bad. But it might be a code smell.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:285
   public:
-ASTUnitStorage(const CompilerInstance &CI);
+ASTUnitStorage(CompilerInstance &CI);
 /// Loads an ASTUnit for a function.

Why is this no longer const?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:120
+case index_error_code::ambiguous_compile_commands_database:
+  return "Compile commands database contains multiple references to the "
+ "same source file.";

Unfortunately, this is a very common case for a large number of projects that 
supports multiple targets (e.g. 32 and 64 bits). Is there a plan to mitigate 
this problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665



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

[clang] 8781944 - [analyzer] GenericTaint: Don't expect CallEvent to always have a Decl.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2020-04-20T15:31:43+03:00
New Revision: 878194414107e94600de31a11be09a347fb2598b

URL: 
https://github.com/llvm/llvm-project/commit/878194414107e94600de31a11be09a347fb2598b
DIFF: 
https://github.com/llvm/llvm-project/commit/878194414107e94600de31a11be09a347fb2598b.diff

LOG: [analyzer] GenericTaint: Don't expect CallEvent to always have a Decl.

This isn't the case when the callee is completely unknown,
eg. when it is a symbolic function pointer.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/test/Analysis/taint-generic.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 1f3e74989229..c06d2fcd8e7d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -110,7 +110,9 @@ class GenericTaintChecker : public Checker {
 
 static Optional create(const CallEvent &Call,
  const CheckerContext &C) {
-  assert(Call.getDecl());
+  if (!Call.getDecl())
+return None;
+
   const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();
   if (!FDecl || (FDecl->getKind() != Decl::Function &&
  FDecl->getKind() != Decl::CXXMethod))

diff  --git a/clang/test/Analysis/taint-generic.c 
b/clang/test/Analysis/taint-generic.c
index a299501b1068..1cc1913eb9a8 100644
--- a/clang/test/Analysis/taint-generic.c
+++ b/clang/test/Analysis/taint-generic.c
@@ -390,3 +390,7 @@ void testConfigurationSinks() {
   mySink(1, 2, x);
   // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}}
 }
+
+void testUnknownFunction(void (*foo)(void)) {
+  foo(); // no-crash
+}



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

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko marked an inline comment as done.
vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

NoQ wrote:
> vsavchenko wrote:
> > NoQ wrote:
> > > I believe this is pretty much always the case. At least whenever 
> > > `getInstanceReceiver()` is available. Another exception seem to be when 
> > > `ReceiverSVal` is an `UnknownVal` (in this case `self` is going to be 
> > > `SymbolRegionValue` because it's never set in the Store), but that's it. 
> > > I inferred this by looking at 
> > > `ObjCMethodCall::getInitialStackFrameContents()`.
> > > 
> > > I think we should have used `getSelfSVal()` to begin with.
> > > I believe this is pretty much always the case.
> > 
> > I didn't quite get what you mean by that
> > 
> > 
> What i'm trying to say is that `C.getSVal(RecE)` and `Message.getSelfSVal()` 
> and `Message.getReceiverSVal()` are basically the same `SVal`. It shouldn't 
> be necessary to check both or check whether they're the same; you must have 
> meant to check for something else, probably something purely syntactic.
> 
> 
> 
> > I inferred this by looking at 
> > ObjCMethodCall::getInitialStackFrameContents().
> 
> Wait, so it's only true for inlined methods. For non-inlined methods 
> `getSelfSVal()` will be unknown :/
Yeah, that might be a bit extraneous to do it with `SVal`s, but this code for 
sure does its job (it is definitely not a redundant check). `getSelfSVal()` 
returns receiver of the function //containing// the call and not the call 
itself. So, it does check if we the receiver of the message is `self`.

I changed it to this way of doing things because it is consistent with how the 
same thing is done in `getRuntimeDefinition`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D78487: Explicitly move from llvm::json Array/Object to Value

[Michael Forster via Phabricator via cfe-commits]

MForster created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
MForster retitled this revision from "Explicitly move from llvm::json Array to 
Value" to "Explicitly move from llvm::json Array/Object to Value".
MForster added a reviewer: sammccall.

The implicit conversion fails under Clang 3.8.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78487

Files:
  clang-tools-extra/clangd/Protocol.cpp


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken &L, const SemanticToken &R) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value &Params, SemanticTokensParams &R) {


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken &L, const SemanticToken &R) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value &Params, SemanticTokensParams &R) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] b36b889 - Explicitly move from llvm::json Array/Object to Value

[Sam McCall via cfe-commits]

Author: Michael Forster
Date: 2020-04-20T15:18:52+02:00
New Revision: b36b889a3b81b893c220c1858d16493152b36852

URL: 
https://github.com/llvm/llvm-project/commit/b36b889a3b81b893c220c1858d16493152b36852
DIFF: 
https://github.com/llvm/llvm-project/commit/b36b889a3b81b893c220c1858d16493152b36852.diff

LOG: Explicitly move from llvm::json Array/Object to Value

Summary: The implicit conversion fails under Clang 3.8.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78487

Added: 


Modified: 
clang-tools-extra/clangd/Protocol.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Protocol.cpp 
b/clang-tools-extra/clangd/Protocol.cpp
index 5756e3b02871..b0c7ce2acb33 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@ static llvm::json::Value 
encodeTokens(llvm::ArrayRef Toks) {
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken &L, const SemanticToken &R) {
@@ -1030,7 +1030,7 @@ llvm::json::Value toJSON(const SemanticTokensOrEdits &TE) 
{
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value &Params, SemanticTokensParams &R) {



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

[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a reviewer: efriedma.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

This is a big patch, but looks reasonable to me.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7451
+  }
+  llvm_unreachable("Unknown MemEltType");
+}

nit: to be consistent, do this in the default clause?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7458
+llvm_unreachable("Invalid SVETypeFlag!");
+
+  case SVETypeFlags::EltTyInt8:

nit: no need for the newlines here and also below?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7555
+
+  // From ACLE we always get . This might be incompatible with
+  // the actual type being loaded. Cast accordingly.

nit: can you rephrase this comment a bit I.e. the "From ACLE we always get ..." 
is a bit confusing I think. You want to say that this is how the ACLE defines 
this, but the IR looks different. You can also specify which bit is different, 
because that was not immediately obvious to me. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77735



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

[PATCH] D77845: [analyzer][CallAndMessage][NFC] Change old callbacks to rely on CallEvent

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:113-114
  const CheckerContext &C) {
-  assert(Call.getDecl());
+  if (!Call.getDecl())
+return None;
   const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();

Well, rG878194414107e94600de31a11be09a347fb2598b got this :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77845



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

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

ostannard added a reviewer: kristof.beyls.
ostannard added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76291



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

[PATCH] D78487: Explicitly move from llvm::json Array/Object to Value

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78487



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

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.
Herald added a subscriber: ASDenysPetrov.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:113
  const CheckerContext &C) {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  assert(Call.getDecl());
+  const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();

rG878194414107e94600de31a11be09a347fb2598b!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72035



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

[PATCH] D76929: [AArch64][SVE] Add SVE intrinsic for LD1RQ

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11622
+  if (VT.isFloatingPoint())
+Load = DAG.getNode(ISD::BITCAST, DL, VT, Load);
+

kmclaughlin wrote:
> sdesmalen wrote:
> > I'd expect this to then use `Load.getValue(0)` ?
> I think this will have the same effect, as `Load` just returns a single value
`SDValue LoadChain = SDValue(Load.getNode(), 1);` suggests that `Load` has two 
return values, the result of the load, and the Chain.



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

https://reviews.llvm.org/D76929



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

[PATCH] D78487: Explicitly move from llvm::json Array/Object to Value

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb36b889a3b81: Explicitly move from llvm::json Array/Object 
to Value (authored by MForster, committed by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78487

Files:
  clang-tools-extra/clangd/Protocol.cpp


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken &L, const SemanticToken &R) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value &Params, SemanticTokensParams &R) {


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken &L, const SemanticToken &R) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value &Params, SemanticTokensParams &R) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 515020c - [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-04-20T14:41:58+01:00
New Revision: 515020c091e74723ee0876229890d71a8aa79702

URL: 
https://github.com/llvm/llvm-project/commit/515020c091e74723ee0876229890d71a8aa79702
DIFF: 
https://github.com/llvm/llvm-project/commit/515020c091e74723ee0876229890d71a8aa79702.diff

LOG: [SveEmitter] Add more immediate operand checks.

This patch adds a number of intrinsics that take immediates with
varying ranges based on the element size one of the operands.

svext:   immediate ranging 0 to (2048/sizeinbits(elt) - 1)
svasrd:  immediate ranging 1..sizeinbits(elt)
svqshlu: immediate ranging 1..sizeinbits(elt)/2
ftmad:   immediate ranging 0..(sizeinbits(elt) - 1)

Reviewers: efriedma, SjoerdMeijer, rovka, rengolin

Reviewed By: SjoerdMeijer

Tags: #clang

Differential Revision: https://reviews.llvm.org/D76679

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_tmad.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_asrd.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_tmad.c
clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qshlu.c
clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_shrnb.c
clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c

Modified: 
clang/include/clang/Basic/arm_sve.td
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/utils/TableGen/SveEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 75fd3ca499d0..9fe4715e4ea1 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -58,9 +58,11 @@
 // ---
 // prototype: return (arg, arg, ...)
 //
+// u: vector of unsigned integers
 // d: default
 // c: const pointer type
 // P: predicate type
+// h: 1/2 width elements, 2x element count
 //
 // i: constant uint64_t
 //
@@ -157,14 +159,18 @@ class ImmCheckType {
 }
 def ImmCheck0_31: ImmCheckType<0>;  // 0..31 (used for e.g. 
predicate patterns)
 def ImmCheck1_16: ImmCheckType<1>;  // 1..16
+def ImmCheckExtract : ImmCheckType<2>;  // 
0..(2048/sizeinbits(elt) - 1)
+def ImmCheckShiftRight  : ImmCheckType<3>;  // 1..sizeinbits(elt)
+def ImmCheckShiftRightNarrow: ImmCheckType<4>;  // 1..sizeinbits(elt)/2
+def ImmCheckShiftLeft   : ImmCheckType<5>;  // 0..(sizeinbits(elt) - 1)
+def ImmCheck0_7 : ImmCheckType<6>;  // 0..7
 
 class ImmCheck {
   int Arg = arg;
-   int EltSizeArg = eltSizeArg;
+  int EltSizeArg = eltSizeArg;
   ImmCheckType Kind = kind;
 }
 
-// Every intrinsic subclasses Inst.
 class Inst ft, list ch, MemEltType met> {
   string Name = n;
@@ -282,6 +288,30 @@ def SVSTNT1 : MInst<"svstnt1[_{d}]", "vPpd", 
"csilUcUsUiUlhfd", [IsStore], MemEl
 // Store one vector, with no truncation, non-temporal (scalar base, VL 
displacement)
 def SVSTNT1_VNUM : MInst<"svstnt1_vnum[_{d}]", "vPpld", "csilUcUsUiUlhfd", 
[IsStore], MemEltTyDefault, "aarch64_sve_stnt1">;
 
+
+// Permutations and selection
+def SVEXT: SInst<"svext[_{d}]",   "dddi", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_ext", [], [ImmCheck<2, ImmCheckExtract, 1>]>;
+
+
+// Shifts
+def SVASRD_M : SInst<"svasrd[_n_{d}]", "dPdi", "csil",MergeOp1,  
"aarch64_sve_asrd", [], [ImmCheck<2, ImmCheckShiftRight, 1>]>;
+
+
+// SVE2 - Narrowing DSP operations
+let ArchGuard = "defined(__ARM_FEATURE_SVE2)" in {
+def SVSHRNB  : SInst<"svshrnb[_n_{d}]","hdi",  "silUsUiUl", MergeNone, 
"aarch64_sve_shrnb", [], [ImmCheck<1, ImmCheckShiftRightNarrow, 0>]>;
+}
+
+
+// SVE2 - Uniform DSP operations
+let ArchGuard = "defined(__ARM_FEATURE_SVE2)" in {
+def SVQSHLU_M  : SInst<"svqshlu[_n_{d}]", "uPdi", "csil", MergeOp1,  
"aarch64_sve_sqshlu", [], [ImmCheck<2, ImmCheckShiftLeft,  1>]>;
+}
+
+
+// Floating-point arithmetic
+def SVTMAD  : SInst<"svtmad[_{d}]",  "dddi", "hfd", MergeNone, 
"aarch64_sve_ftmad_x", [], [ImmCheck<2, ImmCheck0_7>]>;
+
 

 // Saturating scalar arithmetic
 def SVQDECH_S : SInst<"svqdech_pat[_{d}]",   "ddIi", "s", MergeNone, 
"aarch

[PATCH] D78338: [clangd] Enable diagnostic fixes within macro argument expansions.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
+for (auto &FixIt : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&

sammccall wrote:
> hokein wrote:
> > I feel a bit nervous about this (even it is for macro-arg expansion only), 
> > as macro is very tricky.
> > 
> > I think we may result in invalid code after applying the fixits in some 
> > cases:
> > 
> > 1) if the fix is to remove an unused variable (interestingly, clang doesn't 
> > provide fixit to remove an unused variable, but for unused lambda capture, 
> > it does)
> > 
> > ```
> > #define LA(arg1, arg2) [arg1, arg2] { return arg2;}
> > void test1(int x, int y) {
> >   LA(x, y); // after fixit, it would become LA(, y)? or LA(y)
> > }
> > ```
> > 
> > 2) if the fix is to add some characters to the macro argument, e.g. adding 
> > a dereference `*`, the semantic of the code after macro expansion maybe 
> > changed.
> > 
> > ```
> > void f1(int &a);
> > void f2(int *a) {
> >f1(a); // clang will emits a diagnostic with a fixit adding preceding a 
> > `*` to a.
> > }
> > ```
> > 
> > maybe we should be more conservative? just whitelist some diagnostics? 
> > fixing typos seems pretty safe.
> your `test1` example doesn't trigger this case because the fix has to delete 
> a comma that's provided by the macro body - this patch doesn't change 
> behavior.
> 
> To construct an example that follows this schema:
> ```
> struct S { S(int *x); };
> int *x;
> S s1(*x); // fixit -> S s1(x);
> #define CONCAT(a,b) a b
> S s2(CONCAT(*, x)); // fixit -> S s2(CONCAT(, x));
> ```
> 
> The fixed code compiles fine and addresses the error in the expected way. It 
> may not be *idiomatic*, but this is also a pathological case. I think it's at 
> least as good to offer the fix in this case, and certainly it's not a good 
> reason to drop support for others..
> 
> ---
> 
> > void f1(int &a);
> 
> I can't follow this example, there are no macros?
> Why would the insertion change semantics?
> 
> ---
> 
> > maybe we should be more conservative? just whitelist some diagnostics? 
> > fixing typos seems pretty safe.
> 
> I think this leaves a lot of value on the table - we've been conservative so 
> far.
> The problem with whitelists is they're incomplete and outdated (e.g. we have 
> a whitelist for include fixer that's very incomplete, and I haven't managed 
> to get around to fixing it, and neither has anyone else).
> So I think we should use this (or a blacklist) only if we can show this 
> plausibly causes real problems.
> 
> (To put this another way: by being too aggressive we'll get more feedback, by 
> being more conservative we'll continue to get none)
> 
> your test1 example doesn't trigger this case because the fix has to delete a 
> comma that's provided by the macro body - this patch doesn't change behavior.

ah, you are right.

> I can't follow this example, there are no macros?
> Why would the insertion change semantics?

sorry, the example was incomplete, the case is like

```
int f1(int &a);
#define ABC(x) *x + f1(x);
void f2(int *a) {
  ABC(a); // fixit -> ABC(*a), incorrect for the `*x` in macro body.
}
```

if the macro argument is being used in multiple places of the macro body, it 
probably leads to problems. I suspect this is common in practice, we should not 
allow fixit in this case.


>  think this leaves a lot of value on the table - we've been conservative so 
> far.

fair enough.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78338



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

[PATCH] D78401: [SveEmitter] IsInsertOp1SVALL and builtins for svqdec[bhwd] and svqinc[bhwd]

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Looks reasonable




Comment at: clang/include/clang/Basic/arm_sve.td:530
+class sat_type { string U = u; string T = t; }
+def SIGNED_BYTE : sat_type<"",  "c">;
+def SIGNED_HALF : sat_type<"",  "s">;

nit: just wondering if all these defs should be all capitals.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7684
 
+if (TypeFlags.isInsertOp1SVALL())
+  Ops.insert(&Ops[1], Builder.getInt32(31));

would this be the most appropriate place to add the useful sentence from the 
description: 

"Some ACLE builtins leave out the argument to specify the predicate
pattern, which is expected to be expanded to an SV_ALL pattern."

because that's what 31 is, right?



Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_qdecb.c:7
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3

nit: used,unused -> "used/unused", or "used, unused"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78401



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

[PATCH] D78280: [Analyzer][StreamChecker] Track streams that were not found to be opened.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

The high level idea seems great after surveying the analyzer for similar 
issues, but I might need to think about this a bit longer. @baloghadamsoftware, 
IteratorChecker needs to solve similar problems, right? Do you have any input 
on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78280



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

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG515020c091e7: [SveEmitter] Add more immediate operand 
checks. (authored by sdesmalen).

Changed prior to commit:
  https://reviews.llvm.org/D76679?vs=252246&id=258724#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_shrnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -456,6 +456,9 @@
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
+  case 'h':
+ElementBitwidth /= 2;
+break;
   case 'P':
 Signed = true;
 Float = false;
@@ -463,6 +466,11 @@
 Bitwidth = 16;
 ElementBitwidth = 1;
 break;
+  case 'u':
+Predicate = false;
+Signed = false;
+Float = false;
+break;
   case 'i':
 Predicate = false;
 Float = false;
Index: clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svint8_t test_svshrnb_n_s16(svint16_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s16,,)(op1, 0);
+}
+
+svint16_t test_svshrnb_n_s32(svint32_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s32,,)(op1, 0);
+}
+
+svint32_t test_svshrnb_n_s64(svint64_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s64,,)(op1, 0);
+}
+
+svuint8_t test_svshrnb_n_u16(svuint16_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u16,,)(op1, 0);
+}
+
+svuint16_t test_svshrnb_n_u32(svuint32_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u32,,)(op1, 0);
+}
+
+svuint32_t test_svshrnb_n_u64(svuint64_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u64,,)(op1, 0);
+}
Index: clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svuint8_t test_svqshlu_n_s8_m(svbool_t pg, svint8_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}}
+  return SVE_ACLE_FUNC(svqshlu,_n_s8,_m,)(pg, op1, -1);
+}
+
+svuint16_t test_svqshlu_n_s16_m(svbool_t pg, svint16_t

[PATCH] D77594: [SveEmitter] Add support for _n form builtins

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77594



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

[PATCH] D78100: [AST] Suppress the spammy "attempt to use a deleted fucntion" diagnostic.

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12565
+  auto RecoveryExpr =
+  CreateRecoveryExpr(Var->getLocation(), Var->getEndLoc(), {});
+  if (RecoveryExpr.get())

sammccall wrote:
> This seems like it's going to claim some actual tokens, when the thing it 
> represents doesn't cover any tokens.
> 
> I think both start/end source locations should be invalid.
actually, I think it is still valuable to set the var location to recovery-expr.
```
Foo [[foo]]; // if there is a valid default ctor, we have a CtorExpr which has 
the `foo` range; otherwise there is a recoveryExpr with the same range.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78100



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

[clang] fc64539 - [SveEmitter] Add immediate checks for lanes and complex imms

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-04-20T15:10:54+01:00
New Revision: fc645397498037ccb7df230a07e9a8762aaf8c8f

URL: 
https://github.com/llvm/llvm-project/commit/fc645397498037ccb7df230a07e9a8762aaf8c8f
DIFF: 
https://github.com/llvm/llvm-project/commit/fc645397498037ccb7df230a07e9a8762aaf8c8f.diff

LOG: [SveEmitter] Add immediate checks for lanes and complex imms

Adds another bunch of of intrinsics that take immediates with
varying ranges based, some being a complex rotation immediate
which are a set of allowed immediates rather than a range.

svmla_lane:   lane immediate ranging 0..(128/(1*sizeinbits(elt)) - 1)
svcmla_lane:  lane immediate ranging 0..(128/(2*sizeinbits(elt)) - 1)
svdot_lane:   lane immediate ranging 0..(128/(4*sizeinbits(elt)) - 1)
svcadd:   complex rotate immediate [90, 270]
svcmla:
svcmla_lane:  complex rotate immediate [0, 90, 180, 270]

Reviewers: efriedma, SjoerdMeijer, rovka

Reviewed By: efriedma

Tags: #clang

Differential Revision: https://reviews.llvm.org/D76680

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_mla.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cadd.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cmla.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/arm_sve.td
clang/lib/Sema/SemaChecking.cpp
clang/utils/TableGen/SveEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 97ad1a6c7920..a64e313bf271 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9244,6 +9244,10 @@ def err_argument_not_shifted_byte : Error<
   "argument should be an 8-bit value shifted by a multiple of 8 bits">;
 def err_argument_not_shifted_byte_or_xxff : Error<
   "argument should be an 8-bit value shifted by a multiple of 8 bits, or in 
the form 0x??FF">;
+def err_rotation_argument_to_cadd
+: Error<"argument should be the value 90 or 270">;
+def err_rotation_argument_to_cmla
+: Error<"argument should be the value 0, 90, 180 or 270">;
 def warn_neon_vector_initializer_non_portable : Warning<
   "vector initializers are not compatible with NEON intrinsics in big endian "
   "mode">, InGroup>;

diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 9fe4715e4ea1..84f03e60b51f 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -62,7 +62,10 @@
 // d: default
 // c: const pointer type
 // P: predicate type
+// e: 1/2 width unsigned elements, 2x element count
 // h: 1/2 width elements, 2x element count
+// q: 1/4 width elements, 4x element count
+// o: 4x width elements, 1/4 element count
 //
 // i: constant uint64_t
 //
@@ -164,6 +167,11 @@ def ImmCheckShiftRight  : ImmCheckType<3>;  // 
1..sizeinbits(elt)
 def ImmCheckShiftRightNarrow: ImmCheckType<4>;  // 1..sizeinbits(elt)/2
 def ImmCheckShiftLeft   : ImmCheckType<5>;  // 0..(sizeinbits(elt) - 1)
 def ImmCheck0_7 : ImmCheckType<6>;  // 0..7
+def ImmCheckLaneIndex   : ImmCheckType<7>;  // 
0..(128/(1*sizeinbits(elt)) - 1)
+def ImmCheckLaneIndexCompRotate : ImmCheckType<8>;  // 
0..(128/(2*sizeinbits(elt)) - 1)
+def ImmCheckLaneIndexDot: ImmCheckType<9>;  // 
0..(128/(4*sizeinbits(elt)) - 1)
+def ImmCheckComplexRot90_270: ImmCheckType<10>; // [90,270]
+def ImmCheckComplexRotAll90 : ImmCheckType<11>; // [0, 90, 180,270]
 
 class ImmCheck {
   int Arg = arg;
@@ -312,7 +320,19 @@ def SVQSHLU_M  : SInst<"svqshlu[_n_{d}]", "uPdi", "csil",  
   MergeOp1,  "aa
 // Floating-point arithmetic
 def SVTMAD  : SInst<"svtmad[_{d}]",  "dddi", "hfd", MergeNone, 
"aarch64_sve_ftmad_x", [], [ImmCheck<2, ImmCheck0_7>]>;
 
+def SVMLA_LANE  : SInst<"svmla_lane[_{d}]",  "i",  "hfd", MergeNone, 
"aarch64_sve_fmla_lane", [], [ImmCheck<3, ImmCheckLaneIndex, 2>]>;
+def SVCMLA_LANE : SInst<"svcmla_lane[_{d}]", "ii", "hf",  MergeNone, 
"aarch64_sve_fcmla_lane", [], [ImmCheck<3, ImmCheckLaneIndexCompRotate, 2>,
+   
 ImmCheck<4, ImmCheckComplexRotAll90>]>;
+
+def SVCADD_M : SInst<"svcadd[_{d}]", "dPddi",  "hfd", MergeOp1,  
"aarch64_sve_fcadd", [], [ImmCheck<3, ImmCheckComplexRot90_270>]>;
+def SVCMLA_M : SInst<"svcmla[_{d}]", "dPdddi", "hfd", MergeOp1,  
"aarch64_sve_fcmla", [], [ImmCheck<4, ImmCheckComplexRotAll90>]>;
+
 

[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: aykevl, dylanmckay, arsenm, bjope, theraven, 
jrtc27.
Herald added subscribers: cfe-commits, hiraditya, mgorny, wdng.
Herald added a reviewer: ctetreau.
Herald added a project: clang.

APIs such as PointerType::getUnqual/Type::getPointerTo() can result in
pointers being created in address space zero even though this is not
correct for all targets. We have been bitten by this issue many times our
CHERI fork: we use address space 200 for both globals and functions.
Creating a pointer in address space zero will generally result instruction
selection failures or in some cases code being generated that triggers
traps at run time. To avoid these problems, I've remove the many instances
of `unsigned AS = 0` function parameters to ensure that pointers get created
in the right address space.

Keeping these changes out-of-tree results in compilation failures almost
every time I do an upstream merge. It would be very beneficial for us to
have these changes upstreamed, and should help targets such as AVR than
use a non-zero program address space.
I've seen some recent commits for AVR that fix the same AS=0 issues that
we have out-of-tree (e.g. 215dc2e203341f7d1edc4c4a191b048af4ace43d 
).
I think making such bugs a compilation failure should be beneficial for
everyone even if it means typing a few more characters to get a pointer type.

This change allows for gradual migration: we can add a single CMake line
to each directory that should no longer compile with implicit address
space zero: `add_definitions(-DLLVM_NO_IMPLICIT_ADDRESS_SPACE=1)`. In this
patch I've added the definition to lib/IR and I will follow up with further
cleanups.

Depends on D70947 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78491

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.h
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/CodeGen/MachineMemOperand.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/GlobalObject.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Type.h
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/ConstantsContext.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/InlineAsm.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/Mangler.cpp

Index: llvm/lib/IR/Mangler.cpp
===
--- llvm/lib/IR/Mangler.cpp
+++ llvm/lib/IR/Mangler.cpp
@@ -101,7 +101,7 @@
 if (AI->hasByValOrInAllocaAttr())
   Ty = cast(Ty)->getElementType();
 // Size should be aligned to pointer size.
-unsigned PtrSize = DL.getPointerSize();
+unsigned PtrSize = DL.getPointerSize(0);
 ArgWords += alignTo(DL.getTypeAllocSize(Ty), PtrSize);
   }
 
Index: llvm/lib/IR/Instructions.cpp
===
--- llvm/lib/IR/Instructions.cpp
+++ llvm/lib/IR/Instructions.cpp
@@ -616,12 +616,14 @@
   // Create the call to Malloc.
   BasicBlock *BB = InsertBefore ? InsertBefore->getParent() : InsertAtEnd;
   Module *M = BB->getParent()->getParent();
-  Type *BPTy = Type::getInt8PtrTy(BB->getContext());
+  Type *BPTy = Type::getInt8PtrTy(
+  BB->getContext(), M->getDataLayout().getDefaultGlobalsAddressSpace());
   FunctionCallee MallocFunc = MallocF;
   if (!MallocFunc)
 // prototype malloc as "void *malloc(size_t)"
 MallocFunc = M->getOrInsertFunction("malloc", BPTy, IntPtrTy);
-  PointerType *AllocPtrType = PointerType::getUnqual(AllocTy);
+  PointerType *AllocPtrType = AllocTy->getPointerTo(
+  MallocFunc.getFunctionType()->getReturnType()->getPointerAddressSpace());
   CallInst *MCall = nullptr;
   Instruction *Result = nullptr;
   if (InsertBefore) {
@@ -712,7 +714,8 @@
   Module *M = BB->getParent()->getParent();
 
   Type *VoidTy = Type::getVoidTy(M->getContext());
-  Type *IntPtrTy = Type::getInt8PtrTy(M->getContext());
+  Type *IntPtrTy = Type::getInt8PtrTy(
+  M->getContext(), Source->getType()->getPointerAddressSpace());
   // prototype free as "void free(void*)"
   FunctionCallee FreeFunc = M->getOrInsertFunction("free", VoidTy, IntPtrTy);
   CallInst *Result = nullptr;
Index: llvm/lib/IR/InlineAsm.cpp
===
--- llvm/lib/IR/InlineAsm.cpp
+++ llvm/lib/IR/InlineAsm.cpp
@@ -30,7 +30,8 @@
 InlineAsm::InlineAsm(FunctionType *FTy, const std::string &asmString,
  const std::string &constraints, bool hasSideEffects,
  bool isAlignStack, AsmDialect asmDialect)
-: Value(PointerType::getUnqual(FTy), Value::InlineAsmVal),
+: Value(PointerType::get(FTy, FTy->getPointer

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

vsavchenko wrote:
> NoQ wrote:
> > vsavchenko wrote:
> > > NoQ wrote:
> > > > I believe this is pretty much always the case. At least whenever 
> > > > `getInstanceReceiver()` is available. Another exception seem to be when 
> > > > `ReceiverSVal` is an `UnknownVal` (in this case `self` is going to be 
> > > > `SymbolRegionValue` because it's never set in the Store), but that's 
> > > > it. I inferred this by looking at 
> > > > `ObjCMethodCall::getInitialStackFrameContents()`.
> > > > 
> > > > I think we should have used `getSelfSVal()` to begin with.
> > > > I believe this is pretty much always the case.
> > > 
> > > I didn't quite get what you mean by that
> > > 
> > > 
> > What i'm trying to say is that `C.getSVal(RecE)` and 
> > `Message.getSelfSVal()` and `Message.getReceiverSVal()` are basically the 
> > same `SVal`. It shouldn't be necessary to check both or check whether 
> > they're the same; you must have meant to check for something else, probably 
> > something purely syntactic.
> > 
> > 
> > 
> > > I inferred this by looking at 
> > > ObjCMethodCall::getInitialStackFrameContents().
> > 
> > Wait, so it's only true for inlined methods. For non-inlined methods 
> > `getSelfSVal()` will be unknown :/
> Yeah, that might be a bit extraneous to do it with `SVal`s, but this code for 
> sure does its job (it is definitely not a redundant check). `getSelfSVal()` 
> returns receiver of the function //containing// the call and not the call 
> itself. So, it does check if we the receiver of the message is `self`.
> 
> I changed it to this way of doing things because it is consistent with how 
> the same thing is done in `getRuntimeDefinition`.
> `getSelfSVal()` returns receiver of the function containing the call and not 
> the call itself

😱 looks broken to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

[Digger via Phabricator via cfe-commits]

DiggerLin updated this revision to Diff 258732.
DiggerLin marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76932

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-as.c
  llvm/include/llvm/MC/MCAsmInfo.h
  llvm/include/llvm/MC/MCDirectives.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/MC/MCAsmInfoXCOFF.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCXCOFFStreamer.cpp
  llvm/lib/MC/XCOFFObjectWriter.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-LinkOnceAnyLinkage.ll
  llvm/test/CodeGen/PowerPC/aix-LinkOnceODRLinkage.ll
  llvm/test/CodeGen/PowerPC/aix-WeakODRLinkage.ll
  llvm/test/CodeGen/PowerPC/aix-extern-weak.ll
  llvm/test/CodeGen/PowerPC/aix-extern.ll
  llvm/test/CodeGen/PowerPC/aix-reference-func-addr-const.ll
  llvm/test/CodeGen/PowerPC/aix-weak.ll
  llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll

Index: llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
===
--- llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
+++ llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
@@ -36,7 +36,7 @@
 ; OBJ-NEXT:   NumberOfSections: 2
 ; OBJ-NEXT:   TimeStamp: None (0x0)
 ; OBJ-NEXT:   SymbolTableOffset: 0x13C
-; OBJ-NEXT:   SymbolTableEntries: 24
+; OBJ-NEXT:   SymbolTableEntries: 26
 ; OBJ-NEXT:   OptionalHeaderSize: 0x0
 ; OBJ-NEXT:   Flags: 0x0
 ; OBJ-NEXT: }
@@ -86,7 +86,7 @@
 ; RELOC-NEXT:   }
 ; RELOC-NEXT:   Relocation {
 ; RELOC-NEXT: Virtual Address: 0x1A
-; RELOC-NEXT: Symbol: globalA (20)
+; RELOC-NEXT: Symbol: globalA (22)
 ; RELOC-NEXT: IsSigned: No
 ; RELOC-NEXT: FixupBitValue: 0
 ; RELOC-NEXT: Length: 16
@@ -94,7 +94,7 @@
 ; RELOC-NEXT:   }
 ; RELOC-NEXT:   Relocation {
 ; RELOC-NEXT: Virtual Address: 0x1E
-; RELOC-NEXT: Symbol: globalB (22)
+; RELOC-NEXT: Symbol: globalB (24)
 ; RELOC-NEXT: IsSigned: No
 ; RELOC-NEXT: FixupBitValue: 0
 ; RELOC-NEXT: Length: 16
@@ -104,7 +104,7 @@
 ; RELOC-NEXT: Section (index: 2) .data {
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x70
-; RELOC-NEXT:   Symbol: arr (12)
+; RELOC-NEXT:   Symbol: arr (14)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -112,7 +112,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x74
-; RELOC-NEXT:   Symbol: .foo (4)
+; RELOC-NEXT:   Symbol: .foo (6)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -120,7 +120,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x78
-; RELOC-NEXT:   Symbol: TOC (18)
+; RELOC-NEXT:   Symbol: TOC (20)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -128,7 +128,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x80
-; RELOC-NEXT:   Symbol: globalA (8)
+; RELOC-NEXT:   Symbol: globalA (10)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -136,7 +136,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x84
-; RELOC-NEXT:   Symbol: globalB (10)
+; RELOC-NEXT:   Symbol: globalB (12)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -168,6 +168,26 @@
 ; SYM-NEXT:   }
 ; SYM-NEXT:   Symbol {
 ; SYM-NEXT: Index: 2
+; SYM-NEXT: Name: bar
+; SYM-NEXT: Value (RelocatableAddress): 0x0
+; SYM-NEXT: Section: N_UNDEF
+; SYM-NEXT: Type: 0x0
+; SYM-NEXT: StorageClass: C_EXT (0x2)
+; SYM-NEXT: NumberOfAuxEntries: 1
+; SYM-NEXT: CSECT Auxiliary Entry {
+; SYM-NEXT:   Index: 3
+; SYM-NEXT:   SectionLen: 0
+; SYM-NEXT:   ParameterHashIndex: 0x0
+; SYM-NEXT:   TypeChkSectNum: 0x0
+; SYM-NEXT:   SymbolAlignmentLog2: 0
+; SYM-NEXT:   SymbolType: XTY_ER (0x0)
+; SYM-NEXT:   StorageMappingClass: XMC_DS (0xA)
+; SYM-NEXT:   StabInfoIndex: 0x0
+; SYM-NEXT:   StabSectNum: 0x0
+; SYM-NEXT: }
+; SYM-NEXT:   }
+; SYM-NEXT:   Symbol {
+; SYM-NEXT: Index: 4
 ; SYM-NEXT: Name: .text
 ; SYM-NEXT: Value (RelocatableAddress): 0x0
 ; SYM-NEXT: Section: .text
@@ -175,7 +195,7 @@
 ; SYM-NEXT: StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT: NumberOfAuxEntries: 1
 ; SYM-NEXT: CSECT Auxiliary Entry {
-; SYM-NEXT:   Index: 3
+; SYM-NEXT:   Index: 5
 ; SYM-NEXT:   SectionLen: 64
 ; SYM-NEXT:   ParameterHashIndex: 0x0
 ; SYM-NEXT:   TypeChkSectNum: 0x0
@@ -187,7 +207,7 @@
 ; SYM-NEXT: }
 ; SYM-NEXT:   }
 ; SYM-NEXT:   Symbol {
-; SYM-NEXT: Index: 4
+; SYM-NEXT: Index: 6
 ; SYM-NEXT: Name: .foo
 ; SYM-NEXT: Value (RelocatableAddress): 0x0
 ; SYM-NEXT: Section: .text
@@ -195,8 +215,8 @@
 ; SYM-NEXT: StorageClass: C_

[PATCH] D76680: [SveEmitter] Add immediate checks for lanes and complex imms

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sdesmalen marked an inline comment as done.
Closed by commit rGfc6453974980: [SveEmitter] Add immediate checks for lanes 
and complex imms (authored by sdesmalen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76680

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/arm_sve.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_mla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cadd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -456,9 +456,19 @@
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
+  case 'e':
+Signed = false;
+ElementBitwidth /= 2;
+break;
   case 'h':
 ElementBitwidth /= 2;
 break;
+  case 'q':
+ElementBitwidth /= 4;
+break;
+  case 'o':
+ElementBitwidth *= 4;
+break;
   case 'P':
 Signed = true;
 Float = false;
Index: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svfloat16_t test_svmla_lane_f16(svfloat16_t op1, svfloat16_t op2, svfloat16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f16,,)(op1, op2, op3, 8);
+}
+
+svfloat32_t test_svmla_lane_f32(svfloat32_t op1, svfloat32_t op2, svfloat32_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f32,,)(op1, op2, op3, -1);
+}
+
+svfloat64_t test_svmla_lane_f64(svfloat64_t op1, svfloat64_t op2, svfloat64_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f64,,)(op1, op2, op3, 2);
+}
Index: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svint32_t test_svdot_lane_s32(svint32_t op1, svint8_t op2, svint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s32,,)(op1, op2, op3, -1);
+}
+
+svint32_t test_svdot_lane_s32_1(svint32_t op1, svint8_t op2, svint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s32,,)(op1, op2, op3, 4);
+}
+
+svint64_t test_svdot_lane_s64(svint64_t op1, svint16_t op2, svint16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s64,,)(op1, op2, op3, -1);
+}
+
+svint64_t test_svdot_lane_s64_1(svint64_t op1, svint16_t op2, svint16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s64,,)(op1, op2, op3, 2);
+}
+
+svuint32_t test_svdot_lane_u32(svuint32_t op1, svuint8_t op2, svuint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {

[PATCH] D77594: [SveEmitter] Add support for _n form builtins

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Looks reasonable to me




Comment at: clang/utils/TableGen/SveEmitter.cpp:212
+  bool hasSplat() const {
+return Proto.find_first_of("ajfrKLR") != std::string::npos;
+  }

"ajfrKLR" -> bingo ;-)

This probably makes sense, but who knows :-)
Not even sure if a comment makes things better here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77594



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

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield created this revision.
JonChesterfield added reviewers: rjmccall, aaron.ballman, ABataev, jdoerfert, 
arsenm.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.

[nfc] Accept addrspacecast allocas in InitTempAlloca
Changes the precondition to be slightly more permissive. Useful for amdgcn where
allocas are created with a cast to an address space.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78495

Files:
  clang/lib/CodeGen/CGExpr.cpp


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -125,8 +125,12 @@
 }
 
 void CodeGenFunction::InitTempAlloca(Address Var, llvm::Value *Init) {
-  assert(isa(Var.getPointer()));
-  auto *Store = new llvm::StoreInst(Init, Var.getPointer());
+  auto *Alloca = Var.getPointer();
+  assert(isa(Alloca) ||
+ (isa(Alloca) &&
+  isa(
+  cast(Alloca)->getPointerOperand(;
+  auto *Store = new llvm::StoreInst(Init, Alloca);
   Store->setAlignment(Var.getAlignment().getAsAlign());
   llvm::BasicBlock *Block = AllocaInsertPt->getParent();
   Block->getInstList().insertAfter(AllocaInsertPt->getIterator(), Store);


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -125,8 +125,12 @@
 }
 
 void CodeGenFunction::InitTempAlloca(Address Var, llvm::Value *Init) {
-  assert(isa(Var.getPointer()));
-  auto *Store = new llvm::StoreInst(Init, Var.getPointer());
+  auto *Alloca = Var.getPointer();
+  assert(isa(Alloca) ||
+ (isa(Alloca) &&
+  isa(
+  cast(Alloca)->getPointerOperand(;
+  auto *Store = new llvm::StoreInst(Init, Alloca);
   Store->setAlignment(Var.getAlignment().getAsAlign());
   llvm::BasicBlock *Block = AllocaInsertPt->getParent();
   Block->getInstList().insertAfter(AllocaInsertPt->getIterator(), Store);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

[Balázs Benics via Phabricator via cfe-commits]

steakhal marked 2 inline comments as done.
steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:113
  const CheckerContext &C) {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  assert(Call.getDecl());
+  const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();

NoQ wrote:
> rG878194414107e94600de31a11be09a347fb2598b!
Nice catch! Thank you for the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72035



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

[clang] 9986b3d - [SveEmitter] Explicitly merge with zero/undef

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-04-20T16:26:20+01:00
New Revision: 9986b3de26d31be26d978194333c44e82873f3ff

URL: 
https://github.com/llvm/llvm-project/commit/9986b3de26d31be26d978194333c44e82873f3ff
DIFF: 
https://github.com/llvm/llvm-project/commit/9986b3de26d31be26d978194333c44e82873f3ff.diff

LOG: [SveEmitter] Explicitly merge with zero/undef

Builtins that have the merge type MergeAnyExp or MergeZeroExp,
merge into a 'undef' or 'zero' vector respectively, which enables the
_x and _z behaviour for unary operations.

This patch also adds builtins for svabs and svneg.

Reviewers: SjoerdMeijer, efriedma, rovka

Reviewed By: efriedma

Tags: #clang

Differential Revision: https://reviews.llvm.org/D77591

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c

Modified: 
clang/include/clang/Basic/arm_sve.td
clang/lib/CodeGen/CGBuiltin.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 84f03e60b51f..6f665d0c6716 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -296,6 +296,18 @@ def SVSTNT1 : MInst<"svstnt1[_{d}]", "vPpd", 
"csilUcUsUiUlhfd", [IsStore], MemEl
 // Store one vector, with no truncation, non-temporal (scalar base, VL 
displacement)
 def SVSTNT1_VNUM : MInst<"svstnt1_vnum[_{d}]", "vPpld", "csilUcUsUiUlhfd", 
[IsStore], MemEltTyDefault, "aarch64_sve_stnt1">;
 
+
+// Integer arithmetic
+
+multiclass SInstZPZ flags=[]> {
+  def _M : SInst;
+  def _X : SInst;
+  def _Z : SInst;
+}
+
+defm SVABS : SInstZPZ<"svabs", "csil", "aarch64_sve_abs">;
+defm SVNEG : SInstZPZ<"svneg", "csil", "aarch64_sve_neg">;
+
 

 // Permutations and selection
 def SVEXT: SInst<"svext[_{d}]",   "dddi", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_ext", [], [ImmCheck<2, ImmCheckExtract, 1>]>;
@@ -318,6 +330,10 @@ def SVQSHLU_M  : SInst<"svqshlu[_n_{d}]", "uPdi", "csil",  
   MergeOp1,  "aa
 
 

 // Floating-point arithmetic
+
+defm SVABS_F : SInstZPZ<"svabs", "hfd", "aarch64_sve_fabs">;
+defm SVNEG_F : SInstZPZ<"svneg", "hfd", "aarch64_sve_fneg">;
+
 def SVTMAD  : SInst<"svtmad[_{d}]",  "dddi", "hfd", MergeNone, 
"aarch64_sve_ftmad_x", [], [ImmCheck<2, ImmCheck0_7>]>;
 
 def SVMLA_LANE  : SInst<"svmla_lane[_{d}]",  "i",  "hfd", MergeNone, 
"aarch64_sve_fmla_lane", [], [ImmCheck<3, ImmCheckLaneIndex, 2>]>;

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 96c7c9ed2d7b..df45fef9d6c1 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -7591,6 +7591,18 @@ Value *CodeGenFunction::EmitSVEMaskedStore(const 
CallExpr *E,
   return Builder.CreateCall(F, {Val, Predicate, BasePtr});
 }
 
+static void InsertExplicitZeroOperand(CGBuilderTy &Builder, llvm::Type *Ty,
+  SmallVectorImpl &Ops) {
+  auto *SplatZero = Constant::getNullValue(Ty);
+  Ops.insert(Ops.begin(), SplatZero);
+}
+
+static void InsertExplicitUndefOperand(CGBuilderTy &Builder, llvm::Type *Ty,
+   SmallVectorImpl &Ops) {
+  auto *SplatUndef = UndefValue::get(Ty);
+  Ops.insert(Ops.begin(), SplatUndef);
+}
+
 Value *CodeGenFunction::EmitAArch64SVEBuiltinExpr(unsigned BuiltinID,
   const CallExpr *E) {
   // Find out if any arguments are required to be integer constant expressions.
@@ -7630,6 +7642,12 @@ Value 
*CodeGenFunction::EmitAArch64SVEBuiltinExpr(unsigned BuiltinID,
   else if (Builtin->LLVMIntrinsic != 0) {
 llvm::Type* OverloadedTy = getSVEType(TypeFlags);
 
+if (TypeFlags.getMergeType() == SVETypeFlags::MergeZeroExp)
+  InsertExplicitZeroOperand(Builder, Ty, Ops);
+
+if (TypeFlags.getMergeType() == SVETypeFlags::MergeAnyExp)
+  InsertExplicitUndefOperand(Builder, Ty, Ops);
+
 // Predicates must match the main datatype.
 for (unsigned i = 0, e = Ops.size(); i != e; ++i) {
   if (auto PredTy = dyn_cast(Ops[i]->getType()))

diff  --git a/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c 
b/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
new file mode 100644
index ..2db01ff7d64c
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
@@ -0,0 +1,197 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu 
-target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple 
aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-an

[clang] ee12edc - [Preamble] Allow recursive inclusion of header-guarded mainfile.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-04-20T17:28:42+02:00
New Revision: ee12edcb76423c78b55cdddae2edfe45cbb2ccd6

URL: 
https://github.com/llvm/llvm-project/commit/ee12edcb76423c78b55cdddae2edfe45cbb2ccd6
DIFF: 
https://github.com/llvm/llvm-project/commit/ee12edcb76423c78b55cdddae2edfe45cbb2ccd6.diff

LOG: [Preamble] Allow recursive inclusion of header-guarded mainfile.

Summary:
This is guaranteed to be a no-op without the preamble, so should be a
no-op with it too.

Partially fixes https://github.com/clangd/clangd/issues/337
This doesn't yet work for #ifndef guards, which are not recognized in preambles.
see D78038

I can't for the life of me work out how to test this outside clangd.
The original reentrant preamble diagnostic was untested, I added a test
to clangd for that too.

Reviewers: kadircet

Subscribers: ilya-biryukov, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78366

Added: 


Modified: 
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang/lib/Lex/PPDirectives.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 00dd1f864813..f64f8e9901a9 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -33,6 +33,7 @@ using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Pair;
+using testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 ::testing::Matcher WithFix(::testing::Matcher FixMatcher) {
@@ -378,6 +379,47 @@ TEST(DiagnosticsTest, Preprocessor) {
   ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
 }
 
+// Recursive main-file include is diagnosed, and doesn't crash.
+TEST(DiagnosticsTest, RecursivePreamble) {
+  auto TU = TestTU::withCode(R"cpp(
+#include "foo.h" // error-ok
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #pragma once guard is OK.
+TEST(DiagnosticsTest, RecursivePreamblePragmaOnce) {
+  auto TU = TestTU::withCode(R"cpp(
+#pragma once
+#include "foo.h"
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #ifndef guard should be OK.
+// However, it's not yet recognized (incomplete at end of preamble).
+TEST(DiagnosticsTest, RecursivePreambleIfndefGuard) {
+  auto TU = TestTU::withCode(R"cpp(
+#ifndef FOO
+#define FOO
+#include "foo.h" // error-ok
+int symbol;
+#endif
+  )cpp");
+  TU.Filename = "foo.h";
+  // FIXME: should be no errors here.
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
 TEST(DiagnosticsTest, InsideMacros) {
   Annotations Test(R"cpp(
 #define TEN 10

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 660c4a5e089d..d6b6f5695b6c 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1940,19 +1940,6 @@ Preprocessor::ImportAction 
Preprocessor::HandleHeaderIncludeOrImport(
 return {ImportAction::None};
   }
 
-  // Check for circular inclusion of the main file.
-  // We can't generate a consistent preamble with regard to the conditional
-  // stack if the main file is included again as due to the preamble bounds
-  // some directives (e.g. #endif of a header guard) will never be seen.
-  // Since this will lead to confusing errors, avoid the inclusion.
-  if (File && PreambleConditionalStack.isRecording() &&
-  SourceMgr.translateFile(&File->getFileEntry()) ==
-  SourceMgr.getMainFileID()) {
-Diag(FilenameTok.getLocation(),
- diag::err_pp_including_mainfile_in_preamble);
-return {ImportAction::None};
-  }
-
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
@@ -2070,6 +2057,19 @@ Preprocessor::ImportAction 
Preprocessor::HandleHeaderIncludeOrImport(
 Action = (SuggestedModule && !getLangOpts().CompilingPCH) ? Import : Skip;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Sinc

[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7601
+  llvm::Type *SrcDataTy = getSVEType(TypeFlags);
+  llvm::Type *OverloadedTy = llvm::VectorType::get(
+  SVEBuiltinMemEltTy(TypeFlags), SrcDataTy->getVectorElementCount());

nit: This can use `auto`, which would make OverloadedTy a `llvm::VectorType`



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7602
+  llvm::Type *OverloadedTy = llvm::VectorType::get(
+  SVEBuiltinMemEltTy(TypeFlags), SrcDataTy->getVectorElementCount());
+

Just be aware that `getVectorElementCount` has been removed from Type in 
D77278, so you'll need to cast to `VectorType` and use `getElementCount` 
instead.
(fyi - in D77596 I've made `getSVEType` actually return a VectorType so that 
cast wouldn't be needed)



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7635
+  // the actual type being stored. Cast accordingly.
+  Ops[1] = EmitSVEPredicateCast(Ops[1], cast(OverloadedTy));
+

You can remove the `cast` if you make the variable use `auto`.
(these comments apply equally to EmitSVEGatherLoad)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77735



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

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

Needs test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78495



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

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D78495#1992404 , @arsenm wrote:

> Needs test?


I'm not sure how to write said test. How do we normally hit asserts from the 
clang test suite?

This fires a lot in the openmp on amdgcn downstream branch, but I'm happy 
carrying this as a local patch until the rest of the clang change can be put up 
for review if preferred.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78495



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

[PATCH] D77591: [SveEmitter] Explicitly merge with zero/undef

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9986b3de26d3: [SveEmitter] Explicitly merge with zero/undef 
(authored by sdesmalen).

Changed prior to commit:
  https://reviews.llvm.org/D77591?vs=255501&id=258760#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77591

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c

Index: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c
@@ -0,0 +1,197 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+
+#include 
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+svint8_t test_svneg_s8_z(svbool_t pg, svint8_t op)
+{
+  // CHECK-LABEL: test_svneg_s8_z
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv16i8( zeroinitializer,  %pg,  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s8,_z,)(pg, op);
+}
+
+svint16_t test_svneg_s16_z(svbool_t pg, svint16_t op)
+{
+  // CHECK-LABEL: test_svneg_s16_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv8i16( zeroinitializer,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s16,_z,)(pg, op);
+}
+
+svint32_t test_svneg_s32_z(svbool_t pg, svint32_t op)
+{
+  // CHECK-LABEL: test_svneg_s32_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv4i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv4i32( zeroinitializer,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s32,_z,)(pg, op);
+}
+
+svint64_t test_svneg_s64_z(svbool_t pg, svint64_t op)
+{
+  // CHECK-LABEL: test_svneg_s64_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv2i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv2i64( zeroinitializer,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s64,_z,)(pg, op);
+}
+
+svint8_t test_svneg_s8_m(svint8_t inactive, svbool_t pg, svint8_t op)
+{
+  // CHECK-LABEL: test_svneg_s8_m
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv16i8( %inactive,  %pg,  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s8,_m,)(inactive, pg, op);
+}
+
+svint16_t test_svneg_s16_m(svint16_t inactive, svbool_t pg, svint16_t op)
+{
+  // CHECK-LABEL: test_svneg_s16_m
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv8i16( %inactive,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s16,_m,)(inactive, pg, op);
+}
+
+svint32_t test_svneg_s32_m(svint32_t inactive, svbool_t pg, svint32_t op)
+{
+  // CHECK-LABEL: test_svneg_s32_m
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv4i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv4i32( %inactive,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s32,_m,)(inactive, pg, op);
+}
+
+svint64_t test_svneg_s64_m(svint64_t inactive, svbool_t pg, svint64_t op)
+{
+  // CHECK-LABEL: test_svneg_s64_m
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv2i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv2i64( %inactive,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s64,_m,)(inactive, pg, op);
+}
+
+svint8_t test_svneg_s8_x(svbool_t pg, svint8_t op)
+{
+  // CHECK-LABEL: test_svneg_s8_x
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv16i8( undef,  %pg,  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s8,_x,)(pg, op);
+}
+
+svint16_t test_svneg_s16_x(svbool_t pg, svint16_t op)
+{
+  // CHECK-LABEL: test_svneg_s16_x
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv8i16( undef,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s16,_x,)(pg, op);
+}
+
+svint32_t test_svneg_s32_x(svbool_t pg, svint32_t op)
+{
+  // CHECK-L

[PATCH] D78338: [clangd] Enable diagnostic fixes within macro argument expansions.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
+for (auto &FixIt : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&

hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > I feel a bit nervous about this (even it is for macro-arg expansion 
> > > only), as macro is very tricky.
> > > 
> > > I think we may result in invalid code after applying the fixits in some 
> > > cases:
> > > 
> > > 1) if the fix is to remove an unused variable (interestingly, clang 
> > > doesn't provide fixit to remove an unused variable, but for unused lambda 
> > > capture, it does)
> > > 
> > > ```
> > > #define LA(arg1, arg2) [arg1, arg2] { return arg2;}
> > > void test1(int x, int y) {
> > >   LA(x, y); // after fixit, it would become LA(, y)? or LA(y)
> > > }
> > > ```
> > > 
> > > 2) if the fix is to add some characters to the macro argument, e.g. 
> > > adding a dereference `*`, the semantic of the code after macro expansion 
> > > maybe changed.
> > > 
> > > ```
> > > void f1(int &a);
> > > void f2(int *a) {
> > >f1(a); // clang will emits a diagnostic with a fixit adding preceding 
> > > a `*` to a.
> > > }
> > > ```
> > > 
> > > maybe we should be more conservative? just whitelist some diagnostics? 
> > > fixing typos seems pretty safe.
> > your `test1` example doesn't trigger this case because the fix has to 
> > delete a comma that's provided by the macro body - this patch doesn't 
> > change behavior.
> > 
> > To construct an example that follows this schema:
> > ```
> > struct S { S(int *x); };
> > int *x;
> > S s1(*x); // fixit -> S s1(x);
> > #define CONCAT(a,b) a b
> > S s2(CONCAT(*, x)); // fixit -> S s2(CONCAT(, x));
> > ```
> > 
> > The fixed code compiles fine and addresses the error in the expected way. 
> > It may not be *idiomatic*, but this is also a pathological case. I think 
> > it's at least as good to offer the fix in this case, and certainly it's not 
> > a good reason to drop support for others..
> > 
> > ---
> > 
> > > void f1(int &a);
> > 
> > I can't follow this example, there are no macros?
> > Why would the insertion change semantics?
> > 
> > ---
> > 
> > > maybe we should be more conservative? just whitelist some diagnostics? 
> > > fixing typos seems pretty safe.
> > 
> > I think this leaves a lot of value on the table - we've been conservative 
> > so far.
> > The problem with whitelists is they're incomplete and outdated (e.g. we 
> > have a whitelist for include fixer that's very incomplete, and I haven't 
> > managed to get around to fixing it, and neither has anyone else).
> > So I think we should use this (or a blacklist) only if we can show this 
> > plausibly causes real problems.
> > 
> > (To put this another way: by being too aggressive we'll get more feedback, 
> > by being more conservative we'll continue to get none)
> > 
> > your test1 example doesn't trigger this case because the fix has to delete 
> > a comma that's provided by the macro body - this patch doesn't change 
> > behavior.
> 
> ah, you are right.
> 
> > I can't follow this example, there are no macros?
> > Why would the insertion change semantics?
> 
> sorry, the example was incomplete, the case is like
> 
> ```
> int f1(int &a);
> #define ABC(x) *x + f1(x);
> void f2(int *a) {
>   ABC(a); // fixit -> ABC(*a), incorrect for the `*x` in macro body.
> }
> ```
> 
> if the macro argument is being used in multiple places of the macro body, it 
> probably leads to problems. I suspect this is common in practice, we should 
> not allow fixit in this case.
> 
> 
> >  think this leaves a lot of value on the table - we've been conservative so 
> > far.
> 
> fair enough.
> 
> if the macro argument is being used in multiple places of the macro body, it 
> probably leads to problems. I suspect this is common in practice, we should 
> not allow fixit in this case.

This is definitely true.

As discussed offline, this is mitigated by:
 - stringified expansions are common, but don't really count for this purpose
 - fix will often make sense for all occurrences (we can't detect this), in 
which case not offering it is **worse**
 - remaining cases where fix makes sense for one case and not others probably 
aren't that numerous
 - behaviour in this case is to apply the fix and diagnose the resulting error 
from the other expansion, which isn't that terrible (as far as macro diagnostic 
experience goes)

Agreed to do it anyway and wait for feedback, I'll document this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78338



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

[PATCH] D78366: [Preamble] Allow recursive inclusion of header-guarded mainfile.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGee12edcb7642: [Preamble] Allow recursive inclusion of 
header-guarded mainfile. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78366

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/lib/Lex/PPDirectives.cpp

Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1940,19 +1940,6 @@
 return {ImportAction::None};
   }
 
-  // Check for circular inclusion of the main file.
-  // We can't generate a consistent preamble with regard to the conditional
-  // stack if the main file is included again as due to the preamble bounds
-  // some directives (e.g. #endif of a header guard) will never be seen.
-  // Since this will lead to confusing errors, avoid the inclusion.
-  if (File && PreambleConditionalStack.isRecording() &&
-  SourceMgr.translateFile(&File->getFileEntry()) ==
-  SourceMgr.getMainFileID()) {
-Diag(FilenameTok.getLocation(),
- diag::err_pp_including_mainfile_in_preamble);
-return {ImportAction::None};
-  }
-
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
@@ -2070,6 +2057,19 @@
 Action = (SuggestedModule && !getLangOpts().CompilingPCH) ? Import : Skip;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (Action == Enter && File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(&File->getFileEntry()) ==
+  SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_in_preamble);
+return {ImportAction::None};
+  }
+
   if (Callbacks && !IsImportDecl) {
 // Notify the callback object that we've seen an inclusion directive.
 // FIXME: Use a different callback for a pp-import?
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -33,6 +33,7 @@
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Pair;
+using testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 ::testing::Matcher WithFix(::testing::Matcher FixMatcher) {
@@ -378,6 +379,47 @@
   ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
 }
 
+// Recursive main-file include is diagnosed, and doesn't crash.
+TEST(DiagnosticsTest, RecursivePreamble) {
+  auto TU = TestTU::withCode(R"cpp(
+#include "foo.h" // error-ok
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #pragma once guard is OK.
+TEST(DiagnosticsTest, RecursivePreamblePragmaOnce) {
+  auto TU = TestTU::withCode(R"cpp(
+#pragma once
+#include "foo.h"
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #ifndef guard should be OK.
+// However, it's not yet recognized (incomplete at end of preamble).
+TEST(DiagnosticsTest, RecursivePreambleIfndefGuard) {
+  auto TU = TestTU::withCode(R"cpp(
+#ifndef FOO
+#define FOO
+#include "foo.h" // error-ok
+int symbol;
+#endif
+  )cpp");
+  TU.Filename = "foo.h";
+  // FIXME: should be no errors here.
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
 TEST(DiagnosticsTest, InsideMacros) {
   Annotations Test(R"cpp(
 #define TEN 10
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78100: [AST] Suppress the spammy "attempt to use a deleted fucntion" diagnostic.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK LG without changes




Comment at: clang/lib/Sema/SemaDecl.cpp:12565
+  auto RecoveryExpr =
+  CreateRecoveryExpr(Var->getLocation(), Var->getEndLoc(), {});
+  if (RecoveryExpr.get())

hokein wrote:
> sammccall wrote:
> > This seems like it's going to claim some actual tokens, when the thing it 
> > represents doesn't cover any tokens.
> > 
> > I think both start/end source locations should be invalid.
> actually, I think it is still valuable to set the var location to 
> recovery-expr.
> ```
> Foo [[foo]]; // if there is a valid default ctor, we have a CtorExpr which 
> has the `foo` range; otherwise there is a recoveryExpr with the same range.
> ```
Yeah, invalid source range is scary too. Let's go with this and see if things 
break. I think more likely it'll be a mild annoyance like the range of 
CXXConstructExpr.

(In a perfect world maybe this would have a location but no range, or a 
SourceRange would have half-open semantics and could represent a point).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78100



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

[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

[Vojtěch Štěpančík via Phabricator via cfe-commits]

VojtechStep created this revision.
VojtechStep added reviewers: sammccall, chandlerc, Bigcheese.
VojtechStep added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, usaxena95, ormris, kadircet, arphaman, 
dexonsmith, jkorous, MaskRay, ilya-biryukov, hiraditya.
Herald added a project: clang.

This patch adds a function that is similar to 
`llvm::sys::path::home_directory`, but provides access to the system cache 
directory.

For Windows, that is %LOCALAPPDATA%, and applications should put their files 
under %LOCALAPPDATA%\Organization\Product\.

For *nixes, it adheres to the XDG Base Directory Specification, so it first 
looks at the XDG_CACHE_HOME environment variable and falls back to ~/.cache/.

Subsequently, the Clangd Index storage leverages this new API to put index 
files somewhere else than the users home directory.

Previous discussion is here 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78501

Files:
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Unix/Path.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -358,6 +358,136 @@
 }
 #endif
 
+TEST(Support, CacheDirectoryWithEnv) {
+
+  std::string expected;
+
+  // The value of the environment variable is set to a non-standard
+  // location for the test, so we have to store the original value
+  // even if it's not set.
+  // Use an std::string to copy the data
+
+#ifdef _WIN32
+  std::wstring OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", L"C:\\xdg\\cache");
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref{reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, expected);
+  }
+
+  // LocalAppData should always be set, but better safe than sorry
+  if (!expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage.c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  std::string OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  expected = "/xdg/cache";
+  ::setenv("XDG_CACHE_HOME", expected.c_str(), 1);
+  EXPECT_EQ(expected, std::string(::getenv("XDG_CACHE_HOME")));
+
+  {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage.c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
+TEST(Support, CacheDirectoryNoEnv) {
+  std::string expected;
+#ifdef _WIN32
+  std::wstring OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", nullptr);
+  EXPECT_EQ(nullptr, ::_wgetenv("XDG_CACHE_HOME"));
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref{reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, expected);
+  }
+
+  // LocalAppData should always be set, but better safe than sorry
+  if (!expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage.c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  std::string OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::unsetenv("XDG_CACHE_HOME");
+  EXPECT_EQ(nullptr, ::getenv("XDG_CACHE_HOME"));
+
+  SmallString<128> HomeDir;
+  if (path::home_directory(HomeDir)) {
+path::append(HomeDir, ".cache");
+expected = std::string(HomeDir.str());
+  }
+
+  if (!expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage.c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
 TEST(Su

[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7451
+  }
+  llvm_unreachable("Unknown MemEltType");
+}

SjoerdMeijer wrote:
> nit: to be consistent, do this in the default clause?
Doing that would lead to 

  warning: default label in switch which covers all enumeration values 
[-Wcovered-switch-default]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77735



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

[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

[Sam McCall via Phabricator via cfe-commits]

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

Thanks! Looks good with a few nits.

Once you're done, let me know if I should land this for you (after a few 
patches landed this way you can apply for commit access: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)

You can add "Fixes https://github.com/clangd/clangd/issues/341"; to the 
description to close the associated bug.

Note to self: maybe we should add this to the release notes so people can 
delete ~/.clangd.




Comment at: llvm/include/llvm/Support/Path.h:372
+/// Get the directory where installed packages should put their
+/// machine-local cache.
+///

I'd add "e.g. $XDG_CACHE_HOME" to this comment. (No need to spell out the 
fallback or windows behavior, but this gives a bit more of a hint)



Comment at: llvm/include/llvm/Support/Path.h:375
+/// @param result Holds the resulting path name.
+/// @result True if the appropriate directory was found.
+bool cache_directory(SmallVectorImpl &result);

This kind of implies you check that it exists (which it may not, particularly 
on the ~/.cache fallback). Avoiding IO seems like the right thing, but I'd 
mention in the function description that the returned path may not exist yet.



Comment at: llvm/lib/Support/Unix/Path.inc:1142
+bool cache_directory(SmallVectorImpl &result) {
+  char *RequestedDir = getenv("XDG_CACHE_HOME");
+  if (RequestedDir) {

nit: suggest const char* to signal we're not doing anything bad with this 
terrifying API :-)



Comment at: llvm/lib/Support/Unix/Path.inc:1143
+  char *RequestedDir = getenv("XDG_CACHE_HOME");
+  if (RequestedDir) {
+result.clear();

nit: llvm style is to inline this `if (char * RequestedDir = ...)`



Comment at: llvm/unittests/Support/Path.cpp:363
+
+  std::string expected;
+

nit: Expected
(uppercase for vars in llvm style. The style in Path.h/.inc is very old/mixed 
up and you've done a great job being consistent, but at least for local vars in 
the cpp file we should follow the guidelines)



Comment at: llvm/unittests/Support/Path.cpp:426
+#ifdef _WIN32
+  std::wstring OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {

nit: use an llvm::Optional so we correctly distinguish unset vs 
empty?, and elsewhere

(I guess we should extract a RAII environment-restorer for this file, but not 
going to ask you to boil that ocean :-))



Comment at: llvm/unittests/Support/Path.cpp:464
+  SmallString<128> HomeDir;
+  if (path::home_directory(HomeDir)) {
+path::append(HomeDir, ".cache");

you can just ASSERT_TRUE, this should always succeed in our test setups I think.



Comment at: llvm/unittests/Support/Path.cpp:469
+
+  if (!expected.empty()) {
+SmallString<128> CacheDir;

Why are we testing this twice, once conditionally?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78501



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

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Beside Gabors comment I think I'm happy with this. Thanks!




Comment at: clang/unittests/AST/ASTImporterTest.cpp:5939
+
+/// An ExternalASTSource that keeps track of the tags is completed.
+struct SourceWithCompletedTagList : clang::ExternalASTSource {

"is completed" -> "it completed"



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5954
+   SmallVectorImpl &Result) override {
+DC->setHasExternalLexicalStorage(true);
+  }

You can remove this as you changed the check in the ASTImporter to only check 
for the existence of an ExternalASTSource. 


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

https://reviews.llvm.org/D78000



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

[PATCH] D77221: [AVR] Rework MCU family detection to support more AVR MCUs

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Patch generally looks good, but I agree that it could use some basic tests.  
You don't need to do specifically test all 20 million releases, but make sure 
you cover the first and last in the array as well as something in the middle, 
just to make sure your scans are working right.




Comment at: llvm/include/llvm/Support/AVRTargetParser.h:27
+  int Number;
+};
+

I think the relationship would be clearer if this type were named 
`MCUFamilyInfo`.

More generally, these types and fields could use some comments, like 
"Information about a specific AVR microcontroller release" or "Information 
about a family of AVR microcontrollers" or "Some sort of number that goes 
somewhere and has some kind of meaning, maybe it's reported by hardware or 
something, I dunno".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77221



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

[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

I can't give an LGTM but at least from my perspective I very much welcome this 
change. I am still hitting problems with non-zero address spaces on AVR. One 
nit in an inline comment.

Do you plan on eventually removing `LLVM_NO_IMPLICIT_ADDRESS_SPACE` when the 
transition is finished? If so, I think it's worth documenting this somewhere - 
perhaps at llvm/lib/IR/CMakeLists.txt.




Comment at: llvm/include/llvm/IR/DataLayout.h:356
   /// Layout pointer alignment
-  Align getPointerABIAlignment(unsigned AS) const;
 

There is no default address space in this declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78491



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Okay, seems like OpenMP needs unsupported types diagnosing as well. I'm trying 
to adapt this patch for OpenMP, but it doesn't work out of the box because it 
diagnoses memcpy like operations, so with the current patch the code like this 
will cause diagnostics:

   struct T {
 char a;
 __float128 f;
 char c;
 T() : a(12), f(15) {}
  }
  
  #pragma omp declare target
  T a = T();
  #pragma omp end declare target

It happens because member initialization in the constructor is still usage of 
`f` field which is marked unavailable because of type. I'm not sure that it is 
possible to understand how the unavailable declaration is used in the place 
where diagnostic about usage of unavailable declaration is actually emitted, so 
I will probably need some other place/solution for it.

@jdoerfert , could you please help to understand how the diagnostic should work 
for OpenMP cases? Or you probably have some spec/requirements for it?
Understanding what exactly is needed will help with the implementation, I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D78506: [NFC] Common up TargetCodeGenInfo for all PowerPC target.

[Jason Liu via Phabricator via cfe-commits]

jasonliu created this revision.
jasonliu added reviewers: sfertile, nemanjai, hubert.reinterpretcast, 
cebowleratibm.
Herald added subscribers: steven.zhang, shchenz.
jasonliu added a reviewer: PowerPC.
jasonliu added a comment.

To reviewers:
Although the patch is showing changes on various ABIInfo classes. But those 
classes are actually untouched.
This patch only touches PowerPC TargetCodeGenInfo related classes.


There is little differences across all power pc related target for 
TargetCodeGenInfo implementation.
The biggest difference is the different ABIInfo classes it takes. 
But we don't necessarily need so many TargetCodeGenInfo for different PowerPC 
architect. 
So it might be beneficial to common them up as one PPCTargetCodeGenInfo.


https://reviews.llvm.org/D78506

Files:
  clang/lib/CodeGen/TargetInfo.cpp

Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4172,8 +4172,9 @@
   /*allowHigherAlign*/ false);
 }
 
-// PowerPC-32
+// PowerPC
 namespace {
+// PowerPC-32
 /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
@@ -4188,10 +4189,136 @@
 QualType Ty) const override;
 };
 
-class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
+// PowerPC-64
+/// PPC64_SVR4_ABIInfo - The 64-bit PowerPC ELF (SVR4) ABI information.
+class PPC64_SVR4_ABIInfo : public SwiftABIInfo {
+public:
+  enum ABIKind {
+ELFv1 = 0,
+ELFv2
+  };
+
+private:
+  static const unsigned GPRBits = 64;
+  ABIKind Kind;
+  bool HasQPX;
+  bool IsSoftFloatABI;
+
+  // A vector of float or double will be promoted to <4 x f32> or <4 x f64> and
+  // will be passed in a QPX register.
+  bool IsQPXVectorTy(const Type *Ty) const {
+if (!HasQPX)
+  return false;
+
+if (const VectorType *VT = Ty->getAs()) {
+  unsigned NumElements = VT->getNumElements();
+  if (NumElements == 1)
+return false;
+
+  if (VT->getElementType()->isSpecificBuiltinType(BuiltinType::Double)) {
+if (getContext().getTypeSize(Ty) <= 256)
+  return true;
+  } else if (VT->getElementType()->
+   isSpecificBuiltinType(BuiltinType::Float)) {
+if (getContext().getTypeSize(Ty) <= 128)
+  return true;
+  }
+}
+
+return false;
+  }
+
+  bool IsQPXVectorTy(QualType Ty) const {
+return IsQPXVectorTy(Ty.getTypePtr());
+  }
+
+public:
+  PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind, bool HasQPX,
+ bool SoftFloatABI)
+  : SwiftABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
+IsSoftFloatABI(SoftFloatABI) {}
+
+  bool isPromotableTypeForABI(QualType Ty) const;
+  CharUnits getParamTypeAlignment(QualType Ty) const;
+
+  ABIArgInfo classifyReturnType(QualType RetTy) const;
+  ABIArgInfo classifyArgumentType(QualType Ty) const;
+
+  bool isHomogeneousAggregateBaseType(QualType Ty) const override;
+  bool isHomogeneousAggregateSmallEnough(const Type *Ty,
+ uint64_t Members) const override;
+
+  // TODO: We can add more logic to computeInfo to improve performance.
+  // Example: For aggregate arguments that fit in a register, we could
+  // use getDirectInReg (as is done below for structs containing a single
+  // floating-point value) to avoid pushing them to memory on function
+  // entry.  This would require changing the logic in PPCISelLowering
+  // when lowering the parameters in the caller and args in the callee.
+  void computeInfo(CGFunctionInfo &FI) const override {
+if (!getCXXABI().classifyReturnType(FI))
+  FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+for (auto &I : FI.arguments()) {
+  // We rely on the default argument classification for the most part.
+  // One exception:  An aggregate containing a single floating-point
+  // or vector item must be passed in a register if one is available.
+  const Type *T = isSingleElementStruct(I.type, getContext());
+  if (T) {
+const BuiltinType *BT = T->getAs();
+if (IsQPXVectorTy(T) ||
+(T->isVectorType() && getContext().getTypeSize(T) == 128) ||
+(BT && BT->isFloatingPoint())) {
+  QualType QT(T, 0);
+  I.info = ABIArgInfo::getDirectInReg(CGT.ConvertType(QT));
+  continue;
+}
+  }
+  I.info = classifyArgumentType(I.type);
+}
+  }
+
+  Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
+QualType Ty) const override;
+
+  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+  }
+
+  bool isSwiftErrorInRegister() const override {
+return false;
+  }
+};
+
+class PPCTargetCodeGenInf

[PATCH] D73290: [PowerPC] Add clang -msvr4-struct-return for 32-bit ELF

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai accepted this revision.
nemanjai added a comment.

Aside from a couple of minor nits that shouldn't require another review, LGTM.




Comment at: clang/docs/ClangCommandLineReference.rst:2631
+
+Override the default ABI for 32-bit targets to return small structs in
+registers, as in the System V ABI (1995).

Can you specify that "small" means 8 bytes or smaller?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4378
+const llvm::Triple &Triple, const CodeGenOptions &Opts) {
+  assert(Triple.getArch() == llvm::Triple::ppc);
+

Please add text to the assert to help anyone who ends up tripping it. Perhaps:
```
assert(Triple.getArch() == llvm::Triple::ppc &&
   "Invalid triple for a 32-bit PowerPC target");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73290



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

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

NoQ wrote:
> vsavchenko wrote:
> > NoQ wrote:
> > > vsavchenko wrote:
> > > > NoQ wrote:
> > > > > I believe this is pretty much always the case. At least whenever 
> > > > > `getInstanceReceiver()` is available. Another exception seem to be 
> > > > > when `ReceiverSVal` is an `UnknownVal` (in this case `self` is going 
> > > > > to be `SymbolRegionValue` because it's never set in the Store), but 
> > > > > that's it. I inferred this by looking at 
> > > > > `ObjCMethodCall::getInitialStackFrameContents()`.
> > > > > 
> > > > > I think we should have used `getSelfSVal()` to begin with.
> > > > > I believe this is pretty much always the case.
> > > > 
> > > > I didn't quite get what you mean by that
> > > > 
> > > > 
> > > What i'm trying to say is that `C.getSVal(RecE)` and 
> > > `Message.getSelfSVal()` and `Message.getReceiverSVal()` are basically the 
> > > same `SVal`. It shouldn't be necessary to check both or check whether 
> > > they're the same; you must have meant to check for something else, 
> > > probably something purely syntactic.
> > > 
> > > 
> > > 
> > > > I inferred this by looking at 
> > > > ObjCMethodCall::getInitialStackFrameContents().
> > > 
> > > Wait, so it's only true for inlined methods. For non-inlined methods 
> > > `getSelfSVal()` will be unknown :/
> > Yeah, that might be a bit extraneous to do it with `SVal`s, but this code 
> > for sure does its job (it is definitely not a redundant check). 
> > `getSelfSVal()` returns receiver of the function //containing// the call 
> > and not the call itself. So, it does check if we the receiver of the 
> > message is `self`.
> > 
> > I changed it to this way of doing things because it is consistent with how 
> > the same thing is done in `getRuntimeDefinition`.
> > `getSelfSVal()` returns receiver of the function containing the call and 
> > not the call itself
> 
> 😱 looks broken to me.
Let's rename `getSelfSVal()` so that it screamed that it's the callee's self as 
opposed to the caller's self, and explain in a long comment why do we even care 
about the caller's self. I.e., that we heuristically assume that if a class 
method jumps into another class method on the same class object, it's going to 
be devirtualized to the same class. Which isn't always the case, hence !Precise.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D78506: [NFC] Common up TargetCodeGenInfo for all PowerPC target.

[Jason Liu via Phabricator via cfe-commits]

jasonliu added a comment.

To reviewers:
Although the patch is showing changes on various ABIInfo classes. But those 
classes are actually untouched.
This patch only touches PowerPC TargetCodeGenInfo related classes.


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

https://reviews.llvm.org/D78506



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

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

zbrid added a comment.

Gonna land this and file a bug for the failing tests. The tests shouldn't block 
anyone upstream since I'm only adding a build mode. I'll do some more digging 
into those failures in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390



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

[clang] 0f12480 - [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via cfe-commits]

Author: Zola Bridges
Date: 2020-04-20T10:30:52-07:00
New Revision: 0f12480bd13ad2d08b6ef3d64388dd8a12e1a503

URL: 
https://github.com/llvm/llvm-project/commit/0f12480bd13ad2d08b6ef3d64388dd8a12e1a503
DIFF: 
https://github.com/llvm/llvm-project/commit/0f12480bd13ad2d08b6ef3d64388dd8a12e1a503.diff

LOG: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

Summary:
This patch add the dataflow option to LLVM_USE_SANITIZER and documents
it.

Tested via check-cxx (wip to fix the errors).

Reviewers: morehouse, #libc!

Subscribers: mgorny, cfe-commits, libcxx-commits

Tags: #clang, #libc

Differential Revision: https://reviews.llvm.org/D78390

Added: 


Modified: 
clang/docs/DataFlowSanitizer.rst
libcxx/CMakeLists.txt
libcxx/utils/libcxx/test/config.py
llvm/cmake/modules/HandleLLVMOptions.cmake
llvm/docs/CMake.rst

Removed: 




diff  --git a/clang/docs/DataFlowSanitizer.rst 
b/clang/docs/DataFlowSanitizer.rst
index e0e9d74efde9..cc9b8e6e1699 100644
--- a/clang/docs/DataFlowSanitizer.rst
+++ b/clang/docs/DataFlowSanitizer.rst
@@ -20,6 +20,31 @@ specific class of bugs on its own.  Instead, it provides a 
generic
 dynamic data flow analysis framework to be used by clients to help
 detect application-specific issues within their own code.
 
+How to build libc++ with DFSan
+==
+
+DFSan requires either all of your code to be instrumented or for uninstrumented
+functions to be listed as``uninstrumented`` in the `ABI list`_.
+
+If you'd like to have instrumented libc++ functions, then you need to build it
+with DFSan instrumentation from source. Here is an example of how to build
+libc++ and the libc++ ABI with data flow sanitizer instrumentation.
+
+.. code-block:: console
+  cd libcxx-build
+
+  # An example using ninja
+  cmake -GNinja path/to/llvm-project/llvm \
+-DCMAKE_C_COMPILER=clang \
+-DCMAKE_CXX_COMPILER=clang++ \
+-DLLVM_USE_SANITIZER="DataFlow" \
+-DLLVM_ENABLE_LIBCXX=ON \
+-DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi"
+
+  ninja cxx cxxabi
+
+Note: Ensure you are building with a sufficiently new version of Clang.
+
 Usage
 =
 
@@ -33,6 +58,8 @@ The APIs are defined in the header file 
``sanitizer/dfsan_interface.h``.
 For further information about each function, please refer to the header
 file.
 
+.. _ABI list:
+
 ABI List
 
 

diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index cab3f7b14036..b05cad79d76a 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -682,6 +682,8 @@ function(get_sanitizer_flags OUT_VAR  USE_SANITIZER)
   append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined 
-fno-sanitize=vptr,function -fno-sanitize-recover=all")
 elseif (USE_SANITIZER STREQUAL "Thread")
   append_flags(SANITIZER_FLAGS -fsanitize=thread)
+elseif (USE_SANITIZER STREQUAL "DataFlow")
+  append_flags(SANITIZER_FLAGS -fsanitize=dataflow)
 else()
   message(WARNING "Unsupported value of LLVM_USE_SANITIZER: 
${USE_SANITIZER}")
 endif()

diff  --git a/libcxx/utils/libcxx/test/config.py 
b/libcxx/utils/libcxx/test/config.py
index c31a47fb4f5d..ce77ec8c71c9 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -907,6 +907,8 @@ def add_ubsan():
 self.cxx.flags += ['-fsanitize=thread']
 self.config.available_features.add('tsan')
 self.config.available_features.add('sanitizer-new-delete')
+elif san == 'DataFlow':
+self.cxx.flags += ['-fsanitize=dataflow']
 else:
 self.lit_config.fatal('unsupported value for '
   'use_sanitizer: {0}'.format(san))

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake 
b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 0c5f4e08aaba..91133d00782d 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -728,6 +728,8 @@ if(LLVM_USE_SANITIZER)
 elseif (LLVM_USE_SANITIZER STREQUAL "Thread")
   append_common_sanitizer_flags()
   append("-fsanitize=thread" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+elseif (LLVM_USE_SANITIZER STREQUAL "DataFlow")
+  append("-fsanitize=dataflow" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
 LLVM_USE_SANITIZER STREQUAL "Undefined;Address")
   append_common_sanitizer_flags()

diff  --git a/llvm/docs/CMake.rst b/llvm/docs/CMake.rst
index 32d2ebdfc2c2..b8686b6d786e 100644
--- a/llvm/docs/CMake.rst
+++ b/llvm/docs/CMake.rst
@@ -422,7 +422,7 @@ LLVM-specific variables
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, 
``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefin

[clang] 9b2ab41 - Revert "[MS] Fix assert handling enum forward decls in hasVisibleDefinition"

[Rumeet Dhindsa via cfe-commits]

Author: Rumeet Dhindsa
Date: 2020-04-20T10:40:27-07:00
New Revision: 9b2ab41037f45ad92ab4e850591093ffc45d3e10

URL: 
https://github.com/llvm/llvm-project/commit/9b2ab41037f45ad92ab4e850591093ffc45d3e10
DIFF: 
https://github.com/llvm/llvm-project/commit/9b2ab41037f45ad92ab4e850591093ffc45d3e10.diff

LOG: Revert "[MS] Fix assert handling enum forward decls in 
hasVisibleDefinition"

This reverts commit e62dc1f6252c1dcdcc2a64e8e3b07a32412e9d89.

Reverting as per discussion with the patch author.
This patch causes module import error, but there was no intended
behavior change for code that does not use Microsoft extensions.

Added: 


Modified: 
clang/lib/Sema/SemaType.cpp

Removed: 
clang/test/Modules/Inputs/ms-enums/A.h
clang/test/Modules/Inputs/ms-enums/B.h
clang/test/Modules/Inputs/ms-enums/module.map
clang/test/Modules/ms-enums.cpp



diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 4ecd36209e5b..075c30f88b3f 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8120,10 +8120,10 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl 
**Suggested,
   } else if (auto *ED = dyn_cast(D)) {
 if (auto *Pattern = ED->getTemplateInstantiationPattern())
   ED = Pattern;
-if (OnlyNeedComplete && !ED->getIntegerType().isNull()) {
-  // If the enum has an integer type, it may have been forward declared.
-  // Since we're only looking for a complete type (not a definition), any
-  // visible declaration of it will do.
+if (OnlyNeedComplete && ED->isFixed()) {
+  // If the enum has a fixed underlying type, and we're only looking for a
+  // complete type (not a definition), any visible declaration of it will
+  // do.
   *Suggested = nullptr;
   for (auto *Redecl : ED->redecls()) {
 if (isVisible(Redecl))

diff  --git a/clang/test/Modules/Inputs/ms-enums/A.h 
b/clang/test/Modules/Inputs/ms-enums/A.h
deleted file mode 100644
index 168445221c03..
--- a/clang/test/Modules/Inputs/ms-enums/A.h
+++ /dev/null
@@ -1 +0,0 @@
-enum fwd_enum;

diff  --git a/clang/test/Modules/Inputs/ms-enums/B.h 
b/clang/test/Modules/Inputs/ms-enums/B.h
deleted file mode 100644
index 7a13ba4d72d4..
--- a/clang/test/Modules/Inputs/ms-enums/B.h
+++ /dev/null
@@ -1 +0,0 @@
-#include "A.h"

diff  --git a/clang/test/Modules/Inputs/ms-enums/module.map 
b/clang/test/Modules/Inputs/ms-enums/module.map
deleted file mode 100644
index d9aed01430c4..
--- a/clang/test/Modules/Inputs/ms-enums/module.map
+++ /dev/null
@@ -1,2 +0,0 @@
-module A { header "A.h" }
-module B { header "B.h" }

diff  --git a/clang/test/Modules/ms-enums.cpp b/clang/test/Modules/ms-enums.cpp
deleted file mode 100644
index b3a377c6fa63..
--- a/clang/test/Modules/ms-enums.cpp
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions 
-fms-compatibility -x c++ -std=c++20 -fmodules-cache-path=%t -fmodules 
-fimplicit-module-maps -I %S/Inputs/ms-enums %s -verify 
-fno-modules-error-recovery
-
-#include "B.h"
-// expected-note@A.h:1 {{previous declaration is here}}
-// expected-note@A.h:1 2 {{previous definition is here}}
-
-fwd_enum gv_enum; // expected-error {{must be imported}}
-
-struct Foo {
-  enum fwd_enum enum_field; // expected-error 2 {{must be imported}}
-};



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

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

zbrid added a comment.

Bug: https://bugs.llvm.org/show_bug.cgi?id=45621


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390



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

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

zbrid updated this revision to Diff 258789.
zbrid added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390

Files:
  clang/docs/DataFlowSanitizer.rst
  libcxx/CMakeLists.txt
  libcxx/utils/libcxx/test/config.py
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/docs/CMake.rst


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, 
``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -728,6 +728,8 @@
 elseif (LLVM_USE_SANITIZER STREQUAL "Thread")
   append_common_sanitizer_flags()
   append("-fsanitize=thread" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+elseif (LLVM_USE_SANITIZER STREQUAL "DataFlow")
+  append("-fsanitize=dataflow" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
 LLVM_USE_SANITIZER STREQUAL "Undefined;Address")
   append_common_sanitizer_flags()
Index: libcxx/utils/libcxx/test/config.py
===
--- libcxx/utils/libcxx/test/config.py
+++ libcxx/utils/libcxx/test/config.py
@@ -907,6 +907,8 @@
 self.cxx.flags += ['-fsanitize=thread']
 self.config.available_features.add('tsan')
 self.config.available_features.add('sanitizer-new-delete')
+elif san == 'DataFlow':
+self.cxx.flags += ['-fsanitize=dataflow']
 else:
 self.lit_config.fatal('unsupported value for '
   'use_sanitizer: {0}'.format(san))
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -682,6 +682,8 @@
   append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined 
-fno-sanitize=vptr,function -fno-sanitize-recover=all")
 elseif (USE_SANITIZER STREQUAL "Thread")
   append_flags(SANITIZER_FLAGS -fsanitize=thread)
+elseif (USE_SANITIZER STREQUAL "DataFlow")
+  append_flags(SANITIZER_FLAGS -fsanitize=dataflow)
 else()
   message(WARNING "Unsupported value of LLVM_USE_SANITIZER: 
${USE_SANITIZER}")
 endif()
Index: clang/docs/DataFlowSanitizer.rst
===
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -20,6 +20,31 @@
 dynamic data flow analysis framework to be used by clients to help
 detect application-specific issues within their own code.
 
+How to build libc++ with DFSan
+==
+
+DFSan requires either all of your code to be instrumented or for uninstrumented
+functions to be listed as``uninstrumented`` in the `ABI list`_.
+
+If you'd like to have instrumented libc++ functions, then you need to build it
+with DFSan instrumentation from source. Here is an example of how to build
+libc++ and the libc++ ABI with data flow sanitizer instrumentation.
+
+.. code-block:: console
+  cd libcxx-build
+
+  # An example using ninja
+  cmake -GNinja path/to/llvm-project/llvm \
+-DCMAKE_C_COMPILER=clang \
+-DCMAKE_CXX_COMPILER=clang++ \
+-DLLVM_USE_SANITIZER="DataFlow" \
+-DLLVM_ENABLE_LIBCXX=ON \
+-DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi"
+
+  ninja cxx cxxabi
+
+Note: Ensure you are building with a sufficiently new version of Clang.
+
 Usage
 =
 
@@ -33,6 +58,8 @@
 For further information about each function, please refer to the header
 file.
 
+.. _ABI list:
+
 ABI List
 
 


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, ``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules

[PATCH] D78509: [AArch64][SVE] Add addressing mode for contiguous loads & stores

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin created this revision.
kmclaughlin added reviewers: sdesmalen, fpetrogalli, efriedma.
Herald added subscribers: danielkiss, psnobl, rkruppe, hiraditya, 
kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: LLVM.

This patch adds the register + register addressing mode for
SVE contiguous load and store intrinsics (LD1 & ST1)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78509

Files:
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-ld1-addressing-mode-reg-reg.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-ld1.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
===
--- llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
@@ -83,8 +83,7 @@
 define void @st1b_out_of_upper_bound( %data,  %pg, i8* %a) {
 ; CHECK-LABEL: st1b_out_of_upper_bound:
 ; CHECK: rdvl x[[OFFSET:[0-9]+]], #8
-; CHECK: add x[[BASE:[0-9]+]], x0, x[[OFFSET]]
-; CHECK: st1b { z0.b }, p0, [x[[BASE]]]
+; CHECK: st1b { z0.b }, p0, [x0, x[[OFFSET]]]
 ; CHECK-NEXT: ret
   %base_scalable = bitcast i8* %a to *
   %base = getelementptr , * %base_scalable, i64 8
@@ -96,8 +95,7 @@
 define void @st1b_out_of_lower_bound( %data,  %pg, i8* %a) {
 ; CHECK-LABEL: st1b_out_of_lower_bound:
 ; CHECK: rdvl x[[OFFSET:[0-9]+]], #-9
-; CHECK: add x[[BASE:[0-9]+]], x0, x[[OFFSET]]
-; CHECK: st1b { z0.b }, p0, [x[[BASE]]]
+; CHECK: st1b { z0.b }, p0, [x0, x[[OFFSET]]]
 ; CHECK-NEXT: ret
   %base_scalable = bitcast i8* %a to *
   %base = getelementptr , * %base_scalable, i64 -9
Index: llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll
@@ -0,0 +1,184 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; ST1B
+;
+
+define void @st1b_i8( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_i8:
+; CHECK: st1b { z0.b }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,
+   %pred,
+  i8* %base)
+  ret void
+}
+
+
+
+define void @st1b_h( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_h:
+; CHECK: st1b { z0.h }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv8i8( %trunc,
+  %pred,
+ i8* %base)
+  ret void
+}
+
+define void @st1b_s( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_s:
+; CHECK: st1b { z0.s }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv4i8( %trunc,
+  %pred,
+ i8* %base)
+  ret void
+}
+
+define void @st1b_d( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_d:
+; CHECK: st1b { z0.d }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv2i8( %trunc,
+  %pred,
+ i8* %base)
+  ret void
+}
+
+;
+; ST1H
+;
+
+define void @st1h_i16( %data,  %pred, i16* %a, i64 %offset) {
+; CHECK-LABEL: st1h_i16:
+; CHECK: st1h { z0.h }, p0, [x0, x1, lsl #1]
+; CHECK-NEXT: ret
+  %base = getelementptr i16, i16* %a, i64 %offset
+  call void @llvm.aarch64.sve.st1.nxv8i16( %data,
+   %pred,
+  i16* %base)
+  ret void
+}
+
+define void @st1h_f16( %data,  %pred, half* %a, i64 %offset) {
+; CHECK-LABEL: st1h_f16:
+; CHECK: st1h { z0.h }, p0, [x0, x1, lsl #1]
+; CHECK-NEXT: ret
+  %base = getelementptr half, half* %a, i64 %offset
+  call void @llvm.aarch64.sve.st1.nxv8f16( %data,
+   %pred,
+  half* %base)
+  ret void
+}
+
+define void @st1h_s( %data,  %pred, i16* %addr) {
+; CHECK-LABEL: st1h_s:
+; CHECK: st1h { z0.s }, p0, [x0]
+; CHECK-NEXT: ret
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv4i16( %trunc,
+  %pred,
+ i16* %addr)
+  ret void
+}
+
+define void @st1h_d( %data,  %pred, i16* %a, i64 %offset) {
+; CHECK-LABEL: st1h_d:
+; CHECK: st1h { z0.d }, p0, [x0, x1, lsl #1]
+; CHECK-NEXT: ret
+  %base = getelementptr i16, i16* %a, i64 %offset
+  %trunc = trunc  %

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f12480bd13a: [dfsan] Add "DataFlow" option to 
LLVM_USE_SANITIZER (authored by zbrid).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390

Files:
  clang/docs/DataFlowSanitizer.rst
  libcxx/CMakeLists.txt
  libcxx/utils/libcxx/test/config.py
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/docs/CMake.rst


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, 
``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -728,6 +728,8 @@
 elseif (LLVM_USE_SANITIZER STREQUAL "Thread")
   append_common_sanitizer_flags()
   append("-fsanitize=thread" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+elseif (LLVM_USE_SANITIZER STREQUAL "DataFlow")
+  append("-fsanitize=dataflow" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
 LLVM_USE_SANITIZER STREQUAL "Undefined;Address")
   append_common_sanitizer_flags()
Index: libcxx/utils/libcxx/test/config.py
===
--- libcxx/utils/libcxx/test/config.py
+++ libcxx/utils/libcxx/test/config.py
@@ -907,6 +907,8 @@
 self.cxx.flags += ['-fsanitize=thread']
 self.config.available_features.add('tsan')
 self.config.available_features.add('sanitizer-new-delete')
+elif san == 'DataFlow':
+self.cxx.flags += ['-fsanitize=dataflow']
 else:
 self.lit_config.fatal('unsupported value for '
   'use_sanitizer: {0}'.format(san))
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -682,6 +682,8 @@
   append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined 
-fno-sanitize=vptr,function -fno-sanitize-recover=all")
 elseif (USE_SANITIZER STREQUAL "Thread")
   append_flags(SANITIZER_FLAGS -fsanitize=thread)
+elseif (USE_SANITIZER STREQUAL "DataFlow")
+  append_flags(SANITIZER_FLAGS -fsanitize=dataflow)
 else()
   message(WARNING "Unsupported value of LLVM_USE_SANITIZER: 
${USE_SANITIZER}")
 endif()
Index: clang/docs/DataFlowSanitizer.rst
===
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -20,6 +20,31 @@
 dynamic data flow analysis framework to be used by clients to help
 detect application-specific issues within their own code.
 
+How to build libc++ with DFSan
+==
+
+DFSan requires either all of your code to be instrumented or for uninstrumented
+functions to be listed as``uninstrumented`` in the `ABI list`_.
+
+If you'd like to have instrumented libc++ functions, then you need to build it
+with DFSan instrumentation from source. Here is an example of how to build
+libc++ and the libc++ ABI with data flow sanitizer instrumentation.
+
+.. code-block:: console
+  cd libcxx-build
+
+  # An example using ninja
+  cmake -GNinja path/to/llvm-project/llvm \
+-DCMAKE_C_COMPILER=clang \
+-DCMAKE_CXX_COMPILER=clang++ \
+-DLLVM_USE_SANITIZER="DataFlow" \
+-DLLVM_ENABLE_LIBCXX=ON \
+-DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi"
+
+  ninja cxx cxxabi
+
+Note: Ensure you are building with a sufficiently new version of Clang.
+
 Usage
 =
 
@@ -33,6 +58,8 @@
 For further information about each function, please refer to the header
 file.
 
+.. _ABI list:
+
 ABI List
 
 


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, ``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake

[PATCH] D78508: [Clang] Allow long as size_t printf argument on 32-bit Windows platforms.

[Jacek Caban via Phabricator via cfe-commits]

jacek created this revision.
jacek added a reviewer: clang.
Herald added subscribers: cfe-commits, mstorsjo.
Herald added a project: clang.

On 32-bit Windows platform, int and long is mixed all over the place in 
platform headers. The most notable example is SIZE_T, which is unsigned long, 
unlike size_t, which is unsigned int. %I, %z and %j formats do the right thing 
for those types, but clang issues a warning, leaving no good way to print 
SIZE_T (other than disabling warnings or adding useless casts).

GCC supports only %I for mingw targets, but allows both int and long to be used.

This is causing problems when using clang to build Wine PE files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78508

Files:
  clang/lib/AST/FormatString.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/test/Sema/format-strings-ms.c


Index: clang/test/Sema/format-strings-ms.c
===
--- clang/test/Sema/format-strings-ms.c
+++ clang/test/Sema/format-strings-ms.c
@@ -21,17 +21,35 @@
 void signed_test() {
   short val = 30;
   printf("val = %I64d\n", val); // expected-warning{{format specifies type 
'__int64' (aka 'long long') but the argument has type 'short'}}
+  printf("val = %Id\n", val);
   long long bigval = 30;
   printf("val = %I32d\n", bigval); // expected-warning{{format specifies type 
'__int32' (aka 'int') but the argument has type 'long long'}}
   printf("val = %Id\n", bigval); // expected-warning{{format specifies type 
'__int32' (aka 'int') but the argument has type 'long long'}}
+  int ival = 30;
+  printf("%Id", ival);
+  printf("%zd", ival);
+  printf("%td", ival);
+  long lval = 30;
+  printf("%Id", lval);
+  printf("%zd", lval);
+  printf("%td", lval);
 }
 
 void unsigned_test() {
   unsigned short val = 30;
   printf("val = %I64u\n", val); // expected-warning{{format specifies type 
'unsigned __int64' (aka 'unsigned long long') but the argument has type 
'unsigned short'}}
+  printf("val = %Iu\n", val);
   unsigned long long bigval = 30;
   printf("val = %I32u\n", bigval); // expected-warning{{format specifies type 
'unsigned __int32' (aka 'unsigned int') but the argument has type 'unsigned 
long long'}}
   printf("val = %Iu\n", bigval); // expected-warning{{format specifies type 
'unsigned __int32' (aka 'unsigned int') but the argument has type 'unsigned 
long long'}}
+  unsigned int ival = 30;
+  printf("%Iu", ival);
+  printf("%zu", ival);
+  printf("%tu", ival);
+  unsigned long lval = 30;
+  printf("%Iu", lval);
+  printf("%zu", lval);
+  printf("%tu", lval);
 }
 
 void w_test(wchar_t c, wchar_t *s) {
Index: clang/lib/AST/PrintfFormatString.cpp
===
--- clang/lib/AST/PrintfFormatString.cpp
+++ clang/lib/AST/PrintfFormatString.cpp
@@ -527,8 +527,8 @@
 return ArgType::makeSizeT(ArgType(Ctx.getSignedSizeType(), "ssize_t"));
   case LengthModifier::AsInt3264:
 return Ctx.getTargetInfo().getTriple().isArch64Bit()
-   ? ArgType(Ctx.LongLongTy, "__int64")
-   : ArgType(Ctx.IntTy, "__int32");
+   ? ArgType::makeSizeT(ArgType(Ctx.LongLongTy, "__int64"))
+   : ArgType::makeSizeT(ArgType(Ctx.IntTy, "__int32"));
   case LengthModifier::AsPtrDiff:
 return ArgType::makePtrdiffT(
 ArgType(Ctx.getPointerDiffType(), "ptrdiff_t"));
@@ -562,8 +562,10 @@
 return ArgType::makeSizeT(ArgType(Ctx.getSizeType(), "size_t"));
   case LengthModifier::AsInt3264:
 return Ctx.getTargetInfo().getTriple().isArch64Bit()
-   ? ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64")
-   : ArgType(Ctx.UnsignedIntTy, "unsigned __int32");
+   ? ArgType::makeSizeT(
+ ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64"))
+   : ArgType::makeSizeT(
+ ArgType(Ctx.UnsignedIntTy, "unsigned __int32"));
   case LengthModifier::AsPtrDiff:
 return ArgType::makePtrdiffT(
 ArgType(Ctx.getUnsignedPointerDiffType(), "unsigned ptrdiff_t"));
Index: clang/lib/AST/FormatString.cpp
===
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -401,8 +401,21 @@
   case BuiltinType::UInt:
 return T == C.IntTy ? Match : NoMatch;
   case BuiltinType::Long:
+// Win32 platform mixes long and int for size_t-like purposes.
+// ssize_t is int, but platform type SSIZE_T is long.
+if ((isSizeT() || isPtrdiffT()) &&
+C.getTargetInfo().getTriple().isOSMSVCRT() &&
+C.getTargetInfo().getTriple().isArch32Bit())
+  return Match;
 return T == C.UnsignedLongTy ? Match : NoMatch;
   case BuiltinType::ULong:
+// Win32 platform mixes long and int for size_t-like pu

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 258798.
baloghadamsoftware added a comment.

I commented out lines which prevent creation of `StackFrameContext` for 
non-inlined functions. Now everything works regarding the iterator checkers. I 
also simplified things and created new methods for `CallEvent`: 
`getArgObject()` and `getReturnObject()`.

Two tests fail now: `temporaries.cpp` emits `TRUE` in lines `894` and `918` (I 
wonder whether they are correct) and `explain-svals.cpp` emits `lazily frozen 
compound value of parameter ''` in line `96` which is surely incorrect.

The main problem remains to solve is what we already discussed: either to find 
always the same `Decl` or create a new kind of region for arguments.


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

https://reviews.llvm.org/D77229

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/container-modeling.cpp
  clang/test/Analysis/iterator-modeling.cpp

Index: clang/test/Analysis/iterator-modeling.cpp
===
--- clang/test/Analysis/iterator-modeling.cpp
+++ clang/test/Analysis/iterator-modeling.cpp
@@ -1862,7 +1862,7 @@
 void clang_analyzer_printState();
 
 void print_state(std::vector &V) {
-  const auto i0 = V.cbegin();
+  auto i0 = V.cbegin();
   clang_analyzer_printState();
 
 // CHECK:  "checker_messages": [
@@ -1871,7 +1871,8 @@
 // CHECK-NEXT: "i0 : Valid ; Container == SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
 // CHECK-NEXT:   ]}
 
-  const auto i1 = V.cend();
+  ++i0;
+  auto i1 = V.cend();
   clang_analyzer_printState();
   
 // CHECK:  "checker_messages": [
@@ -1879,4 +1880,6 @@
 // CHECK-NEXT: "Iterator Positions :",
 // CHECK-NEXT: "i1 : Valid ; Container == SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
 // CHECK-NEXT:   ]}
+
+  --i1;
 }
Index: clang/test/Analysis/container-modeling.cpp
===
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -17,7 +17,7 @@
 void clang_analyzer_warnIfReached();
 
 void begin(const std::vector &V) {
-  V.begin();
+  const auto i0 = V.begin();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -25,7 +25,7 @@
 }
 
 void end(const std::vector &V) {
-  V.end();
+  const auto i0 = V.end();
 
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
   clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end()}}
@@ -41,10 +41,10 @@
 // Move
 
 void move_assignment(std::vector &V1, std::vector &V2) {
-  V1.cbegin();
-  V1.cend();
-  V2.cbegin();
-  V2.cend();
+  const auto i0 = V1.cbegin();
+  const auto i1 = V1.cend();
+  const auto i2 = V2.cbegin();
+  const auto i3 = V2.cend();
   long B1 = clang_analyzer_container_begin(V1);
   long E1 = clang_analyzer_container_end(V1);
   long B2 = clang_analyzer_container_begin(V2);
@@ -70,8 +70,8 @@
 void clang_analyzer_dump(void*);
 
 void push_back(std::vector &V, int n) {
-  V.cbegin();
-  V.cend();
+  const auto i0 = V.cbegin();
+  const auto i1 = V.cend();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
@@ -90,8 +90,8 @@
 /// past-the-end position of the container is incremented).
 
 void emplace_back(std::vector &V, int n) {
-  V.cbegin();
-  V.cend();
+  const auto i0 = V.cbegin();
+  const auto i1 = V.cend();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
@@ -110,8 +110,8 @@
 /// past-the-end position of the container is decremented).
 
 void pop_back(std::vector &V, int n) {
-  V.cbegin();
-  V.cend();
+  const auto i0 = V.cbegin();
+  const auto i1 = V.cend();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
@@ -131,8 +131,8 @@
 /// position of the container is decremented).
 
 void pus

[PATCH] D78509: [AArch64][SVE] Add addressing mode for contiguous loads & stores

[Francesco Petrogalli via Phabricator via cfe-commits]

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

Hi @kmclaughlin , thank you for working on this!

The patch LGTM, but please consider renaming the multiclass for the non 
faulting loads like I suggested. The rest of the feedback is mostly cosmetic 
changes!

Francesco




Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1617
 
+  multiclass ld1nf {
+// scalar + immediate (mul vl)

The instruction mnemonic is `ldfn1*`, maybe we should call this multiclass 
`ldnf1` instead of `ld1nf`?



Comment at: 
llvm/test/CodeGen/AArch64/sve-intrinsics-ld1-addressing-mode-reg-reg.ll:7
+
+define  @ld1b_i8( %pg, i8* %a, i64 
%offset) {
+; CHECK-LABEL: ld1b_i8

Nit: I think you should rename the variable `%offset` to `%index` all through 
this tests, as usually we intend offset to represent bytes, while index is for 
indexes of arrays, which are scaled.



Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-ld1.ll:1
 ; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
 

For the sake of consistency, I think it is worth splitting this test into two 
tests. One that tests the pattern for the value of offset of 0 (you can leave 
the current name for the file), and one that tests all valid and invalid values 
for the reg+imm addressing mode, and call the test 
`sve-intrinsics-ld1-addressing-mode-reg-imm.ll`



Comment at: 
llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll:7
+
+define void @st1b_i8( %data,  %pred, i8* 
%a, i64 %offset) {
+; CHECK-LABEL: st1b_i8:

Nit: same here. For the sake of consistency with other tests, I think you could 
rename `%offset` to `%index`.



Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll:1
 ; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
 

Nit: Same for this test, I think it is worth splitting for the sake of 
consistency with other tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78509



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

[PATCH] D77156: [CMAKE] Plumb include_directories() into tablegen()

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.
Herald added a subscriber: frgossen.

I'm not sure this is actually the right approach. Adding the directory's 
`INCLUDE_DIRECTORIES` option also adds any path added with 
`include_directories`, which includes system paths and other locations that 
shouldn't ever have tablegen files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77156



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

[PATCH] D77776: [Driver] Default to libc++ on FreeBSD

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

In D6#1981271 , @jbeich wrote:

> - Limit the scope to `-stdlib`
>
> In D6#1972543 , @dim wrote:
>
> > the lowest supported version, which is currently 10
>
>
> Where is this defined? Does someone build LLVM master on FreeBSD < 11.3?


Not certain, but there are a number of FreeBSD consumers still shipping 
products based on FreeBSD 10 who might be trying to use newer toolchains. I 
don't think we need to be too concerned about those cases but if it's trivial 
to avoid breaking them we might as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6



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

[PATCH] D77776: [Driver] Default to libc++ on FreeBSD

[Ed Maste via Phabricator via cfe-commits]

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

Looks ok to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6



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

[PATCH] D78509: [AArch64][SVE] Add addressing mode for contiguous loads & stores

[Francesco Petrogalli via Phabricator via cfe-commits]

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

I think I made a mess with the Actions for this review! I mean to accept it, 
not to enforce the nit comments!

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78509



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

[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

[Vojtěch Štěpančík via Phabricator via cfe-commits]

VojtechStep updated this revision to Diff 258805.
VojtechStep edited the summary of this revision.
VojtechStep added a comment.

@sammccall I think I fixed all raised issues. Let me know if there is anything 
else I can do, otherwise feel free to commit it.

Also Note to self: Once this is pushed to Arch packages we should add Clangd to 
the Supported list of XDG Base Dir Spec support on Arch Wiki 
.


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

https://reviews.llvm.org/D78501

Files:
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Unix/Path.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -358,6 +358,122 @@
 }
 #endif
 
+TEST(Support, CacheDirectoryWithEnv) {
+  std::string Expected;
+  // The value of the environment variable is set to a non-standard
+  // location for the test, so we have to store the original value
+  // even if it's not set.
+  // Use an std::string to copy the data
+#ifdef _WIN32
+  Optional OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", L"C:\\xdg\\cache");
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref{reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, Expected);
+  }
+
+  if (!Expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(Expected, CacheDir);
+  }
+
+  if (OriginalStorage.hasValue()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage->c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  Optional OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  Expected = "/xdg/cache";
+  ::setenv("XDG_CACHE_HOME", Expected.c_str(), 1);
+  EXPECT_EQ(Expected, std::string(::getenv("XDG_CACHE_HOME")));
+
+  SmallString<128> CacheDir;
+  auto status = path::cache_directory(CacheDir);
+  EXPECT_TRUE(status);
+  EXPECT_EQ(Expected, CacheDir);
+
+  if (OriginalStorage.hasValue()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage->c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
+TEST(Support, CacheDirectoryNoEnv) {
+  std::string Expected;
+#ifdef _WIN32
+  Optional OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", nullptr);
+  EXPECT_EQ(nullptr, ::_wgetenv("XDG_CACHE_HOME"));
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref {reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, Expected);
+  }
+
+  // LocalAppData should always be set, but better safe than sorry
+  if (!Expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(Expected, CacheDir);
+  }
+
+  if (OriginalStorage.hasValue()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage->c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  Optional OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::unsetenv("XDG_CACHE_HOME");
+  EXPECT_EQ(nullptr, ::getenv("XDG_CACHE_HOME"));
+
+  SmallString<128> HomeDir;
+
+  auto home_status = path::home_directory(HomeDir);
+  EXPECT_TRUE(home_status);
+  path::append(HomeDir, ".cache");
+  Expected = HomeDir.str().str();
+
+  SmallString<128> CacheDir;
+  auto status = path::cache_directory(CacheDir);
+  EXPECT_TRUE(status);
+  EXPECT_EQ(Expected, CacheDir);
+
+  if (OriginalStorage.hasValue()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage->c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
 TEST(Support, TempDirectory) {
   SmallString<32> TempDir;
   path::system_temp_directory(false, TempDir);
Index: llvm/lib/Support/Windows/Path.inc
===
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -1372,6 +1372,10 @@
   return getKnownFolderPath(FOLDERID_Profile, result);
 }
 
+bool cache_directory(SmallVectorImpl &result) {
+  return getKnownFolderPath(FOLDERID_LocalAppData, result);
+}
+
 static bool getTempDirEnvVar(const wchar_t *Var, SmallVectorImpl &Res) {
   SmallVector Buf;
   size_t Size = 1024;
Index: llvm/lib/Support/Unix/Path.inc

[clang-tools-extra] 6529b0c - [clangd] Enable diagnostic fixes within macro argument expansions.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-04-20T21:18:31+02:00
New Revision: 6529b0c48aab83bdada1d21a79da13b0971d1aec

URL: 
https://github.com/llvm/llvm-project/commit/6529b0c48aab83bdada1d21a79da13b0971d1aec
DIFF: 
https://github.com/llvm/llvm-project/commit/6529b0c48aab83bdada1d21a79da13b0971d1aec.diff

LOG: [clangd] Enable diagnostic fixes within macro argument expansions.

Summary: This seems like a pretty safe case, and common enough to be useful.

Reviewers: hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, 
cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78338

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index 32f6a9bf776c..d72c2bd68ce8 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -556,10 +556,23 @@ void 
StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 if (!InsideMainFile)
   return false;
 
+// Copy as we may modify the ranges.
+auto FixIts = Info.getFixItHints().vec();
 llvm::SmallVector Edits;
-for (auto &FixIt : Info.getFixItHints()) {
-  // Follow clang's behavior, don't apply FixIt to the code in macros,
-  // we are less certain it is the right fix.
+for (auto &FixIt : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  // This can be incorrect if the argument is expanded multiple times in
+  // 
diff erent contexts. Hopefully this is rare!
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&
+  FixIt.RemoveRange.getEnd().isMacroID() &&
+  SM.getFileID(FixIt.RemoveRange.getBegin()) ==
+  SM.getFileID(FixIt.RemoveRange.getEnd())) {
+FixIt.RemoveRange = CharSourceRange(
+{SM.getTopMacroCallerLoc(FixIt.RemoveRange.getBegin()),
+ SM.getTopMacroCallerLoc(FixIt.RemoveRange.getEnd())},
+FixIt.RemoveRange.isTokenRange());
+  }
+  // Otherwise, follow clang's behavior: no fixits in macros.
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 return false;
@@ -570,8 +583,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level 
DiagLevel,
 
 llvm::SmallString<64> Message;
 // If requested and possible, create a message like "change 'foo' to 
'bar'".
-if (SyntheticMessage && Info.getNumFixItHints() == 1) {
-  const auto &FixIt = Info.getFixItHint(0);
+if (SyntheticMessage && FixIts.size() == 1) {
+  const auto &FixIt = FixIts.front();
   bool Invalid = false;
   llvm::StringRef Remove =
   Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, &Invalid);

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index f64f8e9901a9..026e145ed65d 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -104,6 +104,7 @@ TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
 // error-ok
+#define ID(X) X
 namespace test{};
 void $decl[[foo]]();
 class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@ o]]();
   struct Foo { int x; }; Foo a;
   a.$nomember[[y]];
   test::$nomembernamespace[[test]];
+  $macro[[ID($macroarg[[fod]])]]();
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@ o]]();
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+  AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean 
'foo'?"),
+WithFix(Fix(Test.range("macroarg"), "foo",
+"change 'fod' to 'foo'"))),
   // We make sure here that the entire token is highlighted
   AllOf(Diag(Test.range("constructor"),
  "single-argument constructors must be marked explicit to "



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

[clang] 85cca94 - [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

[Erik Pilkington via cfe-commits]

Author: Erik Pilkington
Date: 2020-04-20T15:22:51-04:00
New Revision: 85cca945b4c93ecfd3f3bb7a7b2d5fa4104c3560

URL: 
https://github.com/llvm/llvm-project/commit/85cca945b4c93ecfd3f3bb7a7b2d5fa4104c3560
DIFF: 
https://github.com/llvm/llvm-project/commit/85cca945b4c93ecfd3f3bb7a7b2d5fa4104c3560.diff

LOG: [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

This fixes a common mistake (the 3 should be @3): NSNumber *n = 3. This extends
an existing check for NSString. Also, this only errs if the initializer isn't a
null pointer constant, so NSNumber *n = 0; continues to work. rdar://47029572

Differential revision: https://reviews.llvm.org/D78066

Added: 
clang/test/SemaObjC/objc-literal-fixit.m

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaInit.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a64e313bf271..014ee1c2f2d7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2945,7 +2945,7 @@ def warn_objc_literal_comparison : Warning<
   "a numeric literal|a boxed expression|}0 has undefined behavior">,
   InGroup;
 def err_missing_atsign_prefix : Error<
-  "string literal must be prefixed by '@' ">;
+  "%select{string|numeric}0 literal must be prefixed by '@'">;
 def warn_objc_string_literal_comparison : Warning<
   "direct comparison of a string literal has undefined behavior">,
   InGroup;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index af58b0ec4e82..5cd75b176761 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9455,8 +9455,8 @@ class Sema final {
  QualType DestType, QualType SrcType,
  Expr *&SrcExpr, bool Diagnose = true);
 
-  bool ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&SrcExpr,
-  bool Diagnose = true);
+  bool CheckConversionToObjCLiteral(QualType DstType, Expr *&SrcExpr,
+bool Diagnose = true);
 
   bool checkInitMethod(ObjCMethodDecl *method, QualType receiverTypeIfCall);
 

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fbb5d4b05bbf..75d80c0cc45c 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9283,7 +9283,7 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, 
ExprResult &CallerRHS,
 if (getLangOpts().ObjC &&
 (CheckObjCBridgeRelatedConversions(E->getBeginLoc(), LHSType,
E->getType(), E, Diagnose) ||
- ConversionToObjCStringLiteralCheck(LHSType, E, Diagnose))) {
+ CheckConversionToObjCLiteral(LHSType, E, Diagnose))) {
   if (!Diagnose)
 return Incompatible;
   // Replace the expression with a corrected version and continue so we
@@ -15228,21 +15228,15 @@ ExprResult 
Sema::BuildSourceLocExpr(SourceLocExpr::IdentKind Kind,
   SourceLocExpr(Context, Kind, BuiltinLoc, RPLoc, ParentContext);
 }
 
-bool Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&Exp,
-  bool Diagnose) {
+bool Sema::CheckConversionToObjCLiteral(QualType DstType, Expr *&Exp,
+bool Diagnose) {
   if (!getLangOpts().ObjC)
 return false;
 
   const ObjCObjectPointerType *PT = DstType->getAs();
   if (!PT)
 return false;
-
-  if (!PT->isObjCIdType()) {
-// Check if the destination is the 'NSString' interface.
-const ObjCInterfaceDecl *ID = PT->getInterfaceDecl();
-if (!ID || !ID->getIdentifier()->isStr("NSString"))
-  return false;
-  }
+  const ObjCInterfaceDecl *ID = PT->getInterfaceDecl();
 
   // Ignore any parens, implicit casts (should only be
   // array-to-pointer decays), and not-so-opaque values.  The last is
@@ -15252,15 +15246,41 @@ bool 
Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&Exp,
 if (OV->getSourceExpr())
   SrcExpr = OV->getSourceExpr()->IgnoreParenImpCasts();
 
-  StringLiteral *SL = dyn_cast(SrcExpr);
-  if (!SL || !SL->isAscii())
-return false;
-  if (Diagnose) {
-Diag(SL->getBeginLoc(), diag::err_missing_atsign_prefix)
-<< FixItHint::CreateInsertion(SL->getBeginLoc(), "@");
-Exp = BuildObjCStringLiteral(SL->getBeginLoc(), SL).get();
+  if (auto *SL = dyn_cast(SrcExpr)) {
+if (!PT->isObjCIdType() &&
+!(ID && ID->getIdentifier()->isStr("NSString")))
+  return false;
+if (!SL->isAscii())
+  return false;
+
+if (Diagnose) {
+  Diag(SL->getBeginLoc(), diag::err_missing_atsign_prefix)
+  << /*string

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

You're not testing an assertion, you're testing that code is generated 
correctly for some file on amdgcn.  Just write an ordinary IR-generation test 
that currently crashes and this test fixes.

This is not an NFC change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78495



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

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added a reviewer: dblaikie.

This is a doc change documenting that option -mtune does not perform any CPU 
type specific tuning but exists for compatability reasons with GCC.


https://reviews.llvm.org/D78511

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,9 @@
 def module_file_info : Flag<["-"], "module-file-info">, 
Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accept any  for compatability reasons with GCC, thus not "
+   "performing any CPU type specific tuning">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepts any value for compatability reasons with GCC, thus not performing any 
CPU type specific tuning.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,9 @@
 def module_file_info : Flag<["-"], "module-file-info">, Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accept any  for compatability reasons with GCC, thus not "
+   "performing any CPU type specific tuning">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepts any value for compatability reasons with GCC, thus not performing any CPU type specific tuning.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78338: [clangd] Enable diagnostic fixes within macro argument expansions.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6529b0c48aab: [clangd] Enable diagnostic fixes within macro 
argument expansions. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D78338?vs=258202&id=258812#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78338

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -104,6 +104,7 @@
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
 // error-ok
+#define ID(X) X
 namespace test{};
 void $decl[[foo]]();
 class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@
   struct Foo { int x; }; Foo a;
   a.$nomember[[y]];
   test::$nomembernamespace[[test]];
+  $macro[[ID($macroarg[[fod]])]]();
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+  AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean 
'foo'?"),
+WithFix(Fix(Test.range("macroarg"), "foo",
+"change 'fod' to 'foo'"))),
   // We make sure here that the entire token is highlighted
   AllOf(Diag(Test.range("constructor"),
  "single-argument constructors must be marked explicit to "
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -556,10 +556,23 @@
 if (!InsideMainFile)
   return false;
 
+// Copy as we may modify the ranges.
+auto FixIts = Info.getFixItHints().vec();
 llvm::SmallVector Edits;
-for (auto &FixIt : Info.getFixItHints()) {
-  // Follow clang's behavior, don't apply FixIt to the code in macros,
-  // we are less certain it is the right fix.
+for (auto &FixIt : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  // This can be incorrect if the argument is expanded multiple times in
+  // different contexts. Hopefully this is rare!
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&
+  FixIt.RemoveRange.getEnd().isMacroID() &&
+  SM.getFileID(FixIt.RemoveRange.getBegin()) ==
+  SM.getFileID(FixIt.RemoveRange.getEnd())) {
+FixIt.RemoveRange = CharSourceRange(
+{SM.getTopMacroCallerLoc(FixIt.RemoveRange.getBegin()),
+ SM.getTopMacroCallerLoc(FixIt.RemoveRange.getEnd())},
+FixIt.RemoveRange.isTokenRange());
+  }
+  // Otherwise, follow clang's behavior: no fixits in macros.
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 return false;
@@ -570,8 +583,8 @@
 
 llvm::SmallString<64> Message;
 // If requested and possible, create a message like "change 'foo' to 
'bar'".
-if (SyntheticMessage && Info.getNumFixItHints() == 1) {
-  const auto &FixIt = Info.getFixItHint(0);
+if (SyntheticMessage && FixIts.size() == 1) {
+  const auto &FixIt = FixIts.front();
   bool Invalid = false;
   llvm::StringRef Remove =
   Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, &Invalid);


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -104,6 +104,7 @@
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
 // error-ok
+#define ID(X) X
 namespace test{};
 void $decl[[foo]]();
 class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@
   struct Foo { int x; }; Foo a;
   a.$nomember[[y]];
   test::$nomembernamespace[[test]];
+  $macro[[ID($macroarg[[fod]])]]();
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+  AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean 'foo'?"),
+WithFix(Fix(Test.range("macroarg"), "foo",
+

[PATCH] D78066: [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

[Erik Pilkington via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG85cca945b4c9: [SemaObjC] Forbid storing an unboxed integer 
literal in an NSNumber (authored by erik.pilkington).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78066

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaObjC/objc-literal-fixit.m

Index: clang/test/SemaObjC/objc-literal-fixit.m
===
--- /dev/null
+++ clang/test/SemaObjC/objc-literal-fixit.m
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -verify=c,expected
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -xobjective-c++ -verify=cxx,expected
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -fobjc-arc  -verify=c,arc,expected
+
+typedef signed char BOOL;
+#define YES __objc_yes
+#define NO __objc_no
+
+@interface NSNumber
++(instancetype)numberWithChar:(char)value;
++(instancetype)numberWithInt:(int)value;
++(instancetype)numberWithDouble:(double)value;
++(instancetype)numberWithBool:(BOOL)value;
+@end
+
+void test() {
+  NSNumber *n = YES; // expected-error{{numeric literal must be prefixed by '@'}}
+  NSNumber *n1 = 1; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  NSNumber *n2 = NO; // c-warning{{expression which evaluates to zero treated as a null pointer constant}}
+ // cxx-error@-1{{numeric literal must be prefixed by '@'}}
+  NSNumber *n3 = 0;
+  NSNumber *n4 = 0.0; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  NSNumber *n5 = '\0'; // c-warning{{expression which evaluates to zero treated as a null pointer constant}}
+   // cxx-error@-1{{numeric literal must be prefixed by '@'}}
+
+
+  NSNumber *n6 = '1'; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  int i;
+  NSNumber *n7 = i; // c-warning{{incompatible integer to pointer conversion}}
+// arc-error@-1{{implicit conversion of 'int' to 'NSNumber *' is disallowed with ARC}}
+// cxx-error@-2{{cannot initialize a variable of type 'NSNumber *' with an lvalue of type 'int'}}
+
+  id n8 = 1; // c-warning{{incompatible integer to pointer conversion}}
+ // arc-error@-1{{implicit conversion of 'int' to 'id' is disallowed with ARC}}
+ // cxx-error@-2{{cannot initialize a variable of type 'id' with an rvalue of type 'int'}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5625,7 +5625,7 @@
   if (S.CheckObjCBridgeRelatedConversions(Initializer->getBeginLoc(),
   DestType, Initializer->getType(),
   Initializer) ||
-  S.ConversionToObjCStringLiteralCheck(DestType, Initializer))
+  S.CheckConversionToObjCLiteral(DestType, Initializer))
 Args[0] = Initializer;
 }
 if (!isa(Initializer))
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -4363,7 +4363,7 @@
   // to 'NSString *', instead of falling through to report a "bridge cast"
   // diagnostic.
   if (castACTC == ACTC_retainable && exprACTC == ACTC_none &&
-  ConversionToObjCStringLiteralCheck(castType, castExpr, Diagnose))
+  CheckConversionToObjCLiteral(castType, castExpr, Diagnose))
 return ACR_error;
 
   // Do not issue "bridge cast" diagnostic when implicit casting
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9283,7 +9283,7 @@
 if (getLangOpts().ObjC &&
 (CheckObjCBridgeRelatedConversions(E->getBeginLoc(), LHSType,
E->getType(), E, Diagnose) ||
- ConversionToObjCStringLiteralCheck(LHSType, E, Diagnose))) {
+ CheckConversionToObjCLiteral(LHSType, E, Diagnose))) {
   if (!Diagnose)
 return Incompatible;
   // Replace the expression with a corrected version and continue so we
@@ -15228,21 +15228,15 @@
   SourceLocExpr(Context, Kind, BuiltinLoc, RPLoc, ParentContext);
 }
 
-bool Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&Exp,
-  bool Diagnose) {
+bool Sema::CheckConversionToObjCLiteral(QualType DstType, Expr *&Exp,
+bool Diagnose) {
   if (!getLangOpts().ObjC)
 return false;
 
   const ObjCObjectPointerType *PT

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2738
 
+Accepts any value for compatability reasons with GCC, thus not performing any 
CPU type specific tuning.
+

"Accepts any value, for compatibility with GCC. Does not perform any 
CPU-specific tuning."

In email I said:
> there is no practical difference, but there may be a psychological 
> difference, between saying "Clang permanently treats -mtune as a no-op" 
> versus "Clang has a remarkably low //quality of implementation// for -mtune, 
> and has no immediate plans to either improve or regress it."

To me, this patch's wording implies that Clang is promising to keep -mtune as a 
no-op forever. If Bob puts `-mtune=supercalifragilistic` in his Makefile today, 
and then in the future Clang starts emitting an "unknown target" error, will 
Bob point to this documentation as evidence that Clang promised to accept 
`-mtune=supercalifragilistic` as a no-op?

So I would rather say:

"Accepted for compatibility with GCC. Currently has no effect."


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

https://reviews.llvm.org/D78511



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

[PATCH] D78513: [hip] Claim builtin type `__float128` supported if the host target supports it.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added reviewers: tra, yaxunl.
Herald added subscribers: cfe-commits, kerbowa, nhaehnle, jvesely.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78513

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/SemaCUDA/amdgpu-f128.cu


Index: clang/test/SemaCUDA/amdgpu-f128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-f128.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-unknown-linux-gnu -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef __float128 f128_t;
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -363,4 +363,17 @@
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  // For certain builtin types support on the host target, claim they are
+  // support to pass the compilation of the host code during the device-side
+  // compilation.
+  // FIXME: As the side effect, we also accept `__float128` uses in the device
+  // code. To rejct these builtin types supported in the host target but not in
+  // the device target, one approach would support `device_builtin` attribute
+  // so that we could tell the device builtin types from the host ones. The
+  // also solves the different representations of the same builtin type, such
+  // as `size_t` in the MSVC environment.
+  if (Aux->hasFloat128Type()) {
+HasFloat128 = true;
+Float128Format = DoubleFormat;
+  }
 }


Index: clang/test/SemaCUDA/amdgpu-f128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-f128.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-linux-gnu -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef __float128 f128_t;
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -363,4 +363,17 @@
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  // For certain builtin types support on the host target, claim they are
+  // support to pass the compilation of the host code during the device-side
+  // compilation.
+  // FIXME: As the side effect, we also accept `__float128` uses in the device
+  // code. To rejct these builtin types supported in the host target but not in
+  // the device target, one approach would support `device_builtin` attribute
+  // so that we could tell the device builtin types from the host ones. The
+  // also solves the different representations of the same builtin type, such
+  // as `size_t` in the MSVC environment.
+  if (Aux->hasFloat128Type()) {
+HasFloat128 = true;
+Float128Format = DoubleFormat;
+  }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 258823.
SjoerdMeijer added a comment.

Cheers, that's probably what I wanted to say.


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

https://reviews.llvm.org/D78511

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,8 @@
 def module_file_info : Flag<["-"], "module-file-info">, 
Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accepted for compatibility with GCC. Currently has no effect.">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepted for compatibility with GCC. Currently has no effect.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,8 @@
 def module_file_info : Flag<["-"], "module-file-info">, Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accepted for compatibility with GCC. Currently has no effect.">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepted for compatibility with GCC. Currently has no effect.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits