[all-commits] [llvm/llvm-project] 4446fa: [docs] Fix bullet list formatting

Ye Luo via All-commits all-commits at lists.llvm.org
Mon Feb 20 00:34:41 PST 2023


  Branch: refs/heads/release/16.x
  Home:   https://github.com/llvm/llvm-project
  Commit: 4446fa3c324266d833f3dd74e42844db221272c3
      https://github.com/llvm/llvm-project/commit/4446fa3c324266d833f3dd74e42844db221272c3
  Author: KAWASHIMA Takahiro <t-kawashima at fujitsu.com>
  Date:   2023-02-20 (Mon, 20 Feb 2023)

  Changed paths:
    M clang/docs/LanguageExtensions.rst

  Log Message:
  -----------
  [docs] Fix bullet list formatting

reST requires an empty line before a bullet list.

(cherry picked from commit 6240627cfda444550975ccaea20a3b6d68c9ad15)


  Commit: 80673e368452699b8e8a30f8c2b0c0ce23748228
      https://github.com/llvm/llvm-project/commit/80673e368452699b8e8a30f8c2b0c0ce23748228
  Author: KAWASHIMA Takahiro <t-kawashima at fujitsu.com>
  Date:   2023-02-20 (Mon, 20 Feb 2023)

  Changed paths:
    M clang/docs/LanguageExtensions.rst

  Log Message:
  -----------
  [docs] Update the ACLE URL

(cherry picked from commit e8d44841c5d5f1e7ca2013ab2ae23bb4cec45d1e)


  Commit: 1ab37a25e463ea144289a1cbcf32a7659b2d60bb
      https://github.com/llvm/llvm-project/commit/1ab37a25e463ea144289a1cbcf32a7659b2d60bb
  Author: Balazs Benics <benicsbalazs at gmail.com>
  Date:   2023-02-20 (Mon, 20 Feb 2023)

  Changed paths:
    M clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
    A clang/test/Analysis/symbol-simplification-symplify-results-in-dead-symbol.cpp

  Log Message:
  -----------
  [analyzer] Remove unjustified assert from EquivalenceClass::simplify

One might think that by merging the equivalence classes (eqclasses) of
`Sym1` and `Sym2` symbols we would end up with a `State` in which the
eqclass of `Sym1` and `Sym2` should now be the same. Surprisingly, it's
not //always// true.

Such an example triggered the assertion enforcing this _unjustified_
invariant in https://github.com/llvm/llvm-project/issues/58677.
```lang=C++
unsigned a, b;
#define assert(cond) if (!(cond)) return

void f(unsigned c) {
    /*(1)*/ assert(c == b);
    /*(2)*/ assert((c | a) != a);
    /*(3)*/ assert(a);
    // a = 0  =>  c | 0 != 0  =>  c != 0  =>  b != 0
}
```

I believe, that this assertion hold for reasonable cases - where both
`MemberSym` and `SimplifiedMemberSym` refer to live symbols.
It can only fail if `SimplifiedMemberSym` refers to an already dead
symbol. See the reasoning at the very end.
In this context, I don't know any way of determining if a symbol is
alive/dead, so I cannot refine the assertion in that way.
So, I'm proposing to drop this unjustified assertion.

---

Let me elaborate on why I think the assertion is wrong in its current
shape.

Here is a quick reminder about equivalence classes in CSA.
We have 4 mappings:
1) `ClassMap`: map, associating `Symbols` with an `EquivalenceClass`.
2) `ClassMembers`: map, associating `EquivalenceClasses` with its
   members - basically an enumeration of the `Symbols` which are known
   to be equal.
3) `ConstraintRange`: map, associating `EquivalenceClasses` with the
   range constraint which should hold for all the members of the
   eqclass.
4) `DisequalityMap`: I'm omitting this, as it's irrelevant for our
   purposes now.

Whenever we encounter/assume that two `SymbolRefs` are equal, we update
the `ClassMap` so that now both `SymbolRefs` are referring to the same
eqclass. This operation is known as `merge` or `union`.
Each eqclass is uniquely identified by the `representative` symbol, but
it could have been just a unique number or anything else - the point
is that an eqclass is identified by something unique.
Initially, all Symbols form - by itself - a trivial eqclass, as there
are no other Symbols to which it is assumed to be equal. A trivial
eqclass has //notionally// exactly one member, the representative symbol.
I'm emphasizing that //notionally// because for such cases we don't store
an entry in the `ClassMap` nor in the `ClassMembers` map, because it
could be deduced.

By `merging` two eqclasses, we essentially do what you would think it
does. An important thing to highlight is that the representative symbol
of the resulting eqclass will be the same as one of the two eqclasses.
This operation should be commutative, so that `merge(eq1,eq2)` and
`merge(eq2,eq1)` should result in the same eqclass - except for the
representative symbol, which is just a unique identifier of the class,
a name if you will. Using the representative symbol of `eq1` or `eq2`
should have no visible effect on the analysis overall.

When merging `eq1` into `eq2`, we take each of the `ClassMembers` of
`eq1` and add them to the `ClassMembers` of `eq2` while we also redirect
all the `Symbol` members of `eq1` to map to the `eq2` eqclass in the
`ClassMap`. This way all members of `eq1` will refer to the correct
eqclass. After these, `eq1` key is unreachable in the `ClassMembers`,
hence we can drop it.

---

Let's get back to the example.
Note that when I refer to symbols `a`, `b`, `c`, I'm referring to the
`SymbolRegionValue{VarRegion{.}}` - the value of that variable.

After `(1)`, we will have a constraint `c == b : [1,1]` and an eqclass
`c,b` with the `c` representative symbol.
After `(2)`, we will have an additional constraint `c|b != a : [1,1]`
with the same eqclass. We will also have disequality info about that
`c|a` is disequal to `a` - and the other way around.
However, after the full-expression, `c` will become dead. This kicks in
the garbage collection, which transforms the state into this:
 - We no longer have any constraints, because only `a` is alive, for
   which we don't have any constraints.
 - We have a single (non-trivial) eqclass with a single element `b` and
   representative symbol `c`. (Dead symbols can be representative
   symbols.)
 - We have the same disequality info as before.

At `(3)` within the false branch, `a` get perfectly constrained to zero.
This kicks in the simplification, so we try to substitute (simplify) the
variable in each SymExpr-tree. In our case, it means that the
`c|a != a : [1,1]` constraint gets re-evaluated as `c|0 != 0 : [1,1]`,
which is `c != 0 : [1,1]`.
Under the hood, it means that we will call `merge(c|a, c)`. where `c` is
the result of `simplifyToSVal(State, MemberSym).getAsSymbol()` inside
`EquivalenceClass::simplify()`.
Note that the result of `simplifyToSVal()` was a dead symbol. We
shouldn't have acquired an already dead symbol. AFAIK, this is the only
way we can get one at this point.
Since `c` is dead, we no longer have a mapping in `ClassMap` for it;
hence when we try to `find()` the eqclass of `c`, it will report that
it's a trivial eqclass with the representative symbol `c`.

After `merge(c|a, c)`, we will have a single (non-trivial) eqclass of
`b, c|a` with the representative symbol `c|a` - because we merged the
eqclass of `c` into the eqclass of `c|a`.

This means that `find(c|a)` will result in the eqclass with the
representative symbol `c|a`. So, we ended up having different eqclasses
for `find(c|a)` and `find(c)` after `merge(c|a, c)`, firing the
assertion.

I believe, that this assertion hold for reasonable cases - where both
`MemberSym` and `SimplifiedMemberSym` refer to live symbols.
`MemberSym` should be live in all cases here, because it is from
`ClassMembers` which is pruned in `removeDeadBindings()` so these must
be alive. In this context, I don't know any way of determining if a
symbol is alive/dead, so I cannot refine the assertion in that way.
So, I'm proposing to drop this unjustified assertion.

I'd like to thank @martong for helping me to conclude the investigation.
It was really difficult to track down.

PS: I mentioned that `merge(eq1, eq2)` should be commutative. We
actually exploit this for merging the smaller eqclass into the bigger
one within `EquivalenceClass::merge()`.
Yea, for some reason, if you swap the operands, 3 tests break (only
failures, no crashes) aside from the test which checks the state dumps.
But I believe, that is a different bug and orthogonal to this one. I
just wanted to mention that.
- `Analysis/solver-sym-simplification-adjustment.c`
- `Analysis/symbol-simplification-fixpoint-iteration-unreachable-code.cpp`
- `Analysis/symbol-simplification-reassume.cpp`

Fixes #58677

Reviewed By: vabridgers

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

(cherry picked from commit afcf70aa6de77a0e99f6cb2d3e3824049203d6e2)


  Commit: 6cdd68401c3513584c72826b2c9d4473e0ea026c
      https://github.com/llvm/llvm-project/commit/6cdd68401c3513584c72826b2c9d4473e0ea026c
  Author: R. Voggenauer <rvogg at users.noreply.github.com>
  Date:   2023-02-20 (Mon, 20 Feb 2023)

  Changed paths:
    M llvm/lib/Support/Windows/Path.inc

  Log Message:
  -----------
  [Support] [Windows] Don't check file access time in equivalent(file_status, file_status)

The sys::fs::equivalent(file_status, file_status) function is meant to
judge whether two file_status structs denote the same individual file.
On Unix, this is implemented by only comparing the st_dev and st_ino
numbers (from stat), ignoring all other fields.

On Windows, lacking reliable fields corresponding to st_dev and st_ino,
equivalent(file_status, file_status) compares essentially all fields.
However, since 1e39ef331b2b78baec84fdb577d497511cc46bd5
(https://reviews.llvm.org/D18456), file_status also contains the file
access time.

Including the file access time in equivalent(file_status, file_status)
makes it possible to spuriously break. In particular, when invoking
equivalent(Twine, Twine), with two paths, there's a race condition - the
function calls status() on both input paths. Even if the two paths
are the same, the comparison can fail, if the file was accessed
between the two status() calls.

Thus, it seems like the inclusion of the access time in
equivalent(file_status, file_status) was a mistake.

This race condition can cause spurious failures when linking with
LLD/ELF, where LLD uses equivalent() to check whether a file
exists within a specific sysroot, and that sysroot is accessed by other
processes concurrently.

This fixes downstream issue
https://github.com/msys2/MINGW-packages/issues/15695.

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

(cherry picked from commit a1e80c69223a091e6f0fc84df33a464604c8bbc1)


  Commit: 2e6db258a28b5955d6c885da9c36c4e0a1a17048
      https://github.com/llvm/llvm-project/commit/2e6db258a28b5955d6c885da9c36c4e0a1a17048
  Author: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
  Date:   2023-02-20 (Mon, 20 Feb 2023)

  Changed paths:
    M clang/docs/StandardCPlusPlusModules.rst
    M clang/lib/Frontend/CompilerInstance.cpp
    M clang/lib/Sema/SemaModule.cpp
    M clang/test/CXX/modules-ts/basic/basic.link/module-declaration.cpp
    M clang/test/CXX/modules-ts/dcl.dcl/dcl.module/p2.cpp
    A clang/test/Modules/cxx20-impl-module-conditionally-load.cppm
    A clang/test/Modules/cxx20-named-conditionally-load.cppm

  Log Message:
  -----------
  [C++20] [Modules] Allow -fmodule-file=<module-name>=<BMI-Path> for implementation unit and document the behavior

Close https://github.com/llvm/llvm-project/issues/57293.

Previsouly we can't use `-fmodule-file=<module-name>=<BMI-Path>` for
implementation units, it is a bug. Also the behavior of the above option
is not tested nor documented for C++20 Modules. This patch addresses the
2 problems.

(cherry picked from commit 1782e8f9e882e8f4fb59968ff555c8c93827ea02)


  Commit: f57f59e5aebed3965019e884cc1d60bde97df192
      https://github.com/llvm/llvm-project/commit/f57f59e5aebed3965019e884cc1d60bde97df192
  Author: Joseph Huber <jhuber6 at vols.utk.edu>
  Date:   2023-02-20 (Mon, 20 Feb 2023)

  Changed paths:
    M openmp/libomptarget/include/omptarget.h
    M openmp/libomptarget/src/interface.cpp
    M openmp/libomptarget/src/omptarget.cpp
    M openmp/libomptarget/src/private.h

  Log Message:
  -----------
  [Libomptarget] Check errors when synchronizing the async queue

Summary:
Currently when we synchronize the asynchronous queue for the plugins, we
ignore the return value. This is problematic because we will continue on
like nothing happened if the kernel fails.

Fixes https://github.com/llvm/llvm-project/issues/60814

Reviewed By: jdoerfert

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

(cherry picked from commit 5172877bbddc4e718e4bee57369c820d82f9a784)


  Commit: babeb697ccd7d02a7e5674b3c850c8a8de34c2fe
      https://github.com/llvm/llvm-project/commit/babeb697ccd7d02a7e5674b3c850c8a8de34c2fe
  Author: Ye Luo <yeluo at anl.gov>
  Date:   2023-02-20 (Mon, 20 Feb 2023)

  Changed paths:
    M openmp/libomptarget/include/omptarget.h
    M openmp/libomptarget/src/interface.cpp
    M openmp/libomptarget/src/omptarget.cpp
    M openmp/libomptarget/src/private.h

  Log Message:
  -----------
  [OpenMP] Make isDone lightweight without calling synchronize

~TaskAsyncInfoWrapperTy() calls isDone. With synchronize inside isDone, we need to handle the error return from synchronize in the destructor.
The consumers of TaskAsyncInfoWrapperTy, targetDataMapper and targetKernel, both call AsyncInfo.synchronize() before exiting.
For this reason in ~TaskAsyncInfoWrapperTy(), calling synchronize() via isDone() is redundant.
This patch removes synchronize() call inside isDone() and makes it a lightweight check.
__tgt_target_nowait_query needs to call synchronize() before checking isDone().

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

(cherry picked from commit e2069be83ea88837e1d3bb710aff17cd84ee7b7e)


Compare: https://github.com/llvm/llvm-project/compare/639db8a90d2a...babeb697ccd7


More information about the All-commits mailing list