Skip to content

Conversation

dbartol
Copy link
Owner

@dbartol dbartol commented Sep 12, 2025

The change adds two new properties to each result object in the SARIF log:

partialFingerprints: Contains the "issue hash" that the analyzer already generates for each result, which can help identify a result across runs even if surrounding code changes.

hostedViewUri: If running with -analyzer-format=sarif-html, this property will now be emitted with the file: URL of the generated HTML report for that result.

Along the way, I discovered an existing bug where the HTML diagnostic consumer does not record the path to the generated report if another compilation already created that report. This caused both the SARIF and Plist consumers to be missing the link to the file in all but one of the compilations in case of a warning in a header file. I added a new test to ensure that the generated SARIF for each compilation contains the property.

Finally, I made a few changes to the normalize_sarif processing in the tests. I switched to sed to allow substitutions. The normalization now removes directory components from file: URLs, replaces the length property of the source file with a constant -1, and puts placeholders in the values of the version properties rather than just deleting them. The URL transformation in particular lets us verify that the right filename is generated for each HTML report.

Fixes llvm#158159

rdar://160410408

hazzlim and others added 30 commits September 16, 2025 11:42
Add the following fold AArch64 DAGCombine:
    Fold setcc_merge_zero(
            pred, insert_subvector(undef, signext_inreg(vNi1), 0),
                != splat(0))
-> setcc_merge_zero(pred, insert_subvector(undef, shl(vNi1), 0),
                != splat(0))

as the comparison (!= 0) depends only on bit 0 of the input, the left
shift is sufficient.
…m#158110)

The way that loops strength reduction works is that the target has to
upfront decide whether it wants its addressing to be preindex,
postindex, or neither. This choice affects:
 * Which potential solutions we generate
* Whether we consider a pre/post index load/store as costing an AddRec
or not.

None of these choices are a good fit for either AArch64 or ARM, where
both preindex and postindex addressing are typically free:
* If we pick None then we count pre/post index addressing as costing one
addrec more than is correct so we don't pick them when we should.
* If we pick PreIndexed or PostIndexed then we get the correct cost for
that addressing type, but still get it wrong for the other and also
exclude potential solutions using offset addressing that could have less
cost.

This patch adds an "all" addressing mode that causes all potential
solutions to be generated and counts both pre and postindex as having
AddRecCost of zero. Unfortuntely this reveals problems elsewhere in how
we calculate the cost of things that need to be fixed before we can make
use of it.
…m#157121)

Since `__resize_default_init` is only ever used inside the dylib we can
remove the libc++-internal API and switch to the public one. This patch
inlines a bunch of functions that aren't required anymore and simplifies
the code that way.
…non-index type (llvm#158707)

The current code would crash with integer. This is visible on this
modified example (the original with index was incorrect)
This makes it friendly to -Wglobal-constructors environments. This class
is used when Statistics are disabled, the matching class,
TrackingStatistic, was made constexpr a while back already in
7e5682e for other reasons.
…vm#158667)

Reland llvm#156511 after fixing a build failure not caught by clang. The
default implementation of `parseRecord` is currently unused. Apparently,
clang doesn't type check uninstantiated methods in class templates. To
avoid this footgun, we `= delete` the impl for now.

Original message:

In preparation of larger changes to the bitstream remark format,
refactor the error handling code in the BitstreamRemarkParser.

Main change: move the various static helper methods into the parser
helper classes, so we don't need to pass around as many args. Calling
`error(...)` inside the helper classes now automatically prepends the
current block being parsed to the error message.

NFCI (except for error messages on invalid bitstream files).

Pull Request: llvm#158667
The following commit incorrectly prohibited nested gang loops inside acc
routines. This PR limits the restriction to loops within kernels
constructs only.
llvm@8470027
This patch removes base pointers from subscripts when delinearization
fails. Previously, in such cases, the pointer type SCEVs were used
instead of offset SCEVs derived from them.
For example, here is a portion of the debug output when analyzing
`strong0` in `test/Analysis/DependenceAnalysis/StrongSIV.ll`:

```
testing subscript 0, SIV
    src = {(8 + %A),+,4}<nuw><%for.body>
    dst = {(8 + %A),+,4}<nuw><%for.body>
	Strong SIV test
	    Coeff = 4, i64
	    SrcConst = (8 + %A), ptr
	    DstConst = (8 + %A), ptr
	    Delta = 0, i64
	    UpperBound = (-1 + %n), i64
	    Distance = 0
	    Remainder = 0
```

As shown above, the `SrcConst` and `DstConst` are pointer values rather
than integer offsets. `%A` should be removed.

This change is necessary for llvm#157086, since
`ScalarEvolution::willNotOverflow` expects integer type SCEVs as
arguments. This change alone alone should not affect the analysis
results.
In order to avoid recalculating stride of strided load twice save it in
a map.
…158674)

* Use streams to avoid dealing with std::string
* Print operand masks in hex
* Make the output more succinct
This PR refactors alignment validation in MLIR's MemRef and SPIRV
dialects:
- Use `IntValidAlignment` for consistent type safety across MemRef and
SPIRV dialects
  - Eliminate duplicate validation logic in `MemRefOps.cpp`
  - Adjust error messages in `invalid.mlir` to match improved validation
  
This is the first of two PRs addressing issue llvm#155677.
The refactoring lead to an additional data transfer. This changes the
assumed transfers in the check-strings to work with that changed
behavior.
Summary:
Currently we have this `__tgt_device_image` indirection which just takes
a reference to some pointers. This was all find and good when the only
usage of this was from a section of GPU code that came from an ELF
constant section. However, we have expanded beyond that and now need to
worry about managing lifetimes. We have code that references the image
even after it was loaded internally. This patch changes the
implementation to instaed copy the memory buffer and manage it locally.

This PR reworks the JIT and other image handling to directly manage its
own memory. We now don't need to duplicate this behavior externally at
the Offload API level. Also we actually free these if the user unloads
them.

Upside, less likely to crash and burn. Downside, more latency when
loading an image.
…vm#159103)

PR llvm#159045 made the constructor constexpr, which allows
`-Wunused-variable` to trigger. However, we don't really care if a
statistic is unused if `LLVM_ENABLE_STATS` is 0.
…rn a string (NFC) (llvm#159089)

These functions will see more uses in a future patch.
This also resolves a FIXME.
…onDAG (llvm#155256)

Based on comment of
llvm#153600 (comment),
Add a helper function isTailCall for getting libcall in SelectionDAG.
…et (llvm#158597)

Fixes llvm#157252.
Peephole optimization tends to fold:
```
add %gpr1, %stack, 0
subs %gpr2, %gpr1, 0
```
to
```
adds %gpr2, %stack, 0
```

This patch undoes the fold in `rewriteAArch64FrameIndex` to process
`adds` on the stack object.
Fixes an issue in commit 3946c50, PR llvm#135349.

The DebugSSAUpdater class performs raw pointer allocations. It frees
these properly in reset(), but does not do so in its destructor - as an
immediate fix, this patch adds a destructor which frees the allocations
correctly.

I'll be merging this immediately to fix the issue, but will be open to
post-commit review and/or producing a better fix in a follow-up commit.
The S_NOP instruction has an immediate operand which is one less than
the number of cycles to delay for. The maximum value that may be encoded
in this field was increased in GFX8 and again in GFX12.
…vm#158026)

IR has the `contract` to indicate that contraction is allowed. Testing
shouldn't rely on global flag to perform contraction. This is a
prerequisite before making backends rely only on the IR to perform
contraction. See more here:
https://discourse.llvm.org/t/allowfpopfusion-vs-sdnodeflags-hasallowcontract/80909/5
dbartol and others added 29 commits September 17, 2025 09:25
When `-analyzer-output=sarif-html` is specified, the analyzer was
reporting each warning to the console three times. This is because the
code to create the diagnostic consumers for `sarif-html` was calling the
code for `sarif` and `html` separately, each of which also creates its
own console text consumer. Then the `sarif-html` code itself created a
third.

The fix is to factor out the creation of the SARIF and HTML consumers
from the text consumers, so `sarif-html` just calls the code to create
the SARIF and HTML consumers without the text consumers.

The same fix applies for `plist-html`.

I've updated one of the SARIF tests to specify `sarif-html`. This test
would fail in the regular `-verify` validation due to the triplicated
warnings, but now passes with my fix.

Fixes llvm#158103 
rdar://160383710
…lvm#157762)

Clang would complain about conflicting types between a function
declaration and definition if the declaraion was marked with the
attribute but the definition wasn't. Do not treat this as an error. It
should only be necessary to mark the declaration with the attribute.
This PR emits and relaxes the FREs generated in the previous PR.

After this change llvm emits usable sframe sections that can be
linked with the gnu linker. There are a few remaining cfi directives to 
handle before they are generally usable, however.

The output isn't identical with gnu-gas in every case (this code
produces
fewer identical FREs in a row than gas), but I'm reasonably sure that
they are correct regardless. There are even more size optimizations that
can be done later.

Also, while working on the tests, I found a few bugs in older portions
and cleaned those up.

This is a fairly big commit, but I'm not sure how to make it smaller.
…lvm#159395)

Since llvm#157170 this test has been flakey on several LLDB buildbots. I
suspect it's to do with mutli-threading, there are more details in
llvm#159377.

Disable parallel loading for now so we are not spamming people making
unrelated changes.
GlobalMerge has been enabled for minsize for a while, this patch enables
it more generally. In my testing it did not affect performance very
much, especially with the linker relaxations we already perform, but
should help reduce code size a little.
…mpl. (llvm#158558)

As far as I understand, GISelValueTracking::computeKnownBitsImpl does
not need to be virtual, as no-one is specializing / overriding it, which
means it is not necessary for the destructor either.
As a follow up to llvm#159122, after figure out reason why CAS unit tests
are failing on Solaris, update the CMake configuration to build ondisk
CAS implementation. We now check the existance of `flock` before
enabling the configuration.

In the future, we can find ways to support OnDiskCAS on other platforms
that do not have `flock`. This can techinically be done with a POSIX
compilant file lock but that will put a restriction on the usage of CAS.
Removes resource constructors that take binding information per proposal update llvm/wg-hlsl#336. The constructors are replaced by static `__createFromBinding` and `__createFromImplicitBinding` methods on the resource class.
Reverts llvm#131804.

This breaks a build bot; see discussion in the original PR. I could
reproduce this problem locally. The problem is that the original PR
makes use of `registerCustomChecks` in `ClangTidy.cpp` but does not add
the new custom module to the dependencies of the target that builds that
file. Adding the dependency would create a cyclic dependency, so the fix
doesn't seem obvious. See details in the PR description.
* Remove `getOperandValuesImpl` since its only used once.

* Extract common logic from
`DeadCodeAnalysis::visitRegion{BranchOperation,Terminator}` into a new
function `DeadCodeAnalysis::visitRegionBranchEdges`.

  In particular, both functions do the following:

  * Detect live region branch edges (similar to CFGEdge);

* For each edge, mark the successor program point as executable (so that
subsequent program gets visited);

* For each edge, store the information of the predecessor op and
arguments (so that other analyses know what states to join into the
successor program point).

One caveat is that, before this PR, in `visitRegionTerminator`, the
successor program point is only marked as live if it is the start of a
block; after this PR, the successor program point is consistently marked
as live regardless what it is, which makes the behavior equal to
`visitBranchOperation`. This minor fix improves consistency, but at this
point it is still NFC, because the rest of the dataflow analysis
framework only cares about liveness at block level, and the liveness
information in the middle of a block isn't read anyway. This probably
will change once
[early-exits](https://discourse.llvm.org/t/rfc-region-based-control-flow-with-early-exits-in-mlir/76998)
are supported.
…59123)

The RENAME intrinsic did not correctly remove trailing spaces from
filenames. This PR introduces code to remove trailing blanks as
documented by GFortran.

---------

Co-authored-by: Copilot <[email protected]>
This PR builds on the llvm#158314
and adds the lowering support for `-gdwarf-N` flag. The changes to pass
the information to `AddDebugInfo` pass are mostly mechanical. The
`AddDebugInfo` pass adds `ModuleFlagsOp` in the module which gets
translated to correct llvm metadata during mlir->llvmir translation.

There is minor correction where the version is set to 0 in case no
-debug-version flag is provided. Previously it was set to 2 in this case
due to misreading of clang code.
There is a number of attributes that is expected to be set on functions
by default. This patch implements setting more such attributes on the
FMV resolver functions generated by Clang. On AArch64, this makes the
resolver functions use the default PAC and BTI hardening settings.
…vm#159312)

This popped up during our internal integrates of upstream changes. It
started happening after ba9d1c4, which
started using `TemplateSpecializationType` in this place and the code
was not prepared to handle it.
…gs. NFC. (llvm#159327)

This avoids the following kind of warning with GCC:

    warning: control reaches end of non-void function [-Wreturn-type]
llvm#159337)

This fixes the following warning when compiled with GCC:

    ../lib/Target/AArch64/AArch64ISelLowering.cpp: In function ‘bool shouldLowerTailCallStackArg(const llvm::MachineFunction&, const llvm::CCValAssign&, llvm::SDValue, llvm::ISD::ArgFlagsTy, int)’:
    ../lib/Target/AArch64/AArch64ISelLowering.cpp:9310: warning: comparison of integer expressions of different signedness: ‘uint64_t’ {aka ‘long unsigned int’} and ‘int64_t’ {aka ‘long int’} [-Wsign-compare]
     9310 |       if (SizeInBits / 8 != MFI.getObjectSize(FI))
          |
)

This avoids the following warnings from Clang:

    ../../lldb/source/Host/windows/Host.cpp:324:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]
      324 |   default:
          |   ^
    ../../lldb/source/Host/common/File.cpp:662:26: warning: cast from 'const void *' to 'char *' drops const qualifier [-Wcast-qual]
      662 |           .write((char *)buf, num_bytes);
          |                          ^
…ngs. NFC. (llvm#159330)

This avoids the following warnings:

    ../../clang/lib/AST/ExprConstant.cpp: In member function ‘bool {anonymous}::IntExprEvaluator::VisitBuiltinCallExpr(const clang::CallExpr*, unsigned int)’:
    ../../clang/lib/AST/ExprConstant.cpp:14104:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
    14104 |   }
          |   ^
    ../../clang/lib/AST/ExprConstant.cpp:14105:3: note: here
    14105 |   case Builtin::BIstrlen:
          |   ^~~~
    ../../clang/lib/Driver/ToolChains/CommonArgs.cpp: In function ‘std::string clang::driver::tools::complexRangeKindToStr(clang::LangOptionsBase::ComplexRangeKind ’:
    ../../clang/lib/Driver/ToolChains/CommonArgs.cpp:3523:1: warning: control reaches end of non-void function [-Wreturn-type]
     3523 | }
          | ^
…59333)

This avoids the following kind of warning with GCC:

    ../tools/llvm-lipo/llvm-lipo.cpp: In function ‘void printInfo(llvm::LLVMContext&, llvm::ArrayRef<llvm::object::OwningBinary<llvm::object::Binary> >)’:
    ../tools/llvm-lipo/llvm-lipo.cpp:464:34: warning: suggest parentheses around ‘& ’ within ‘||’ [-Wparentheses]
      464 |              Binary->isArchive() && "expected MachO binary");
          |              ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
While debugging llvm#145206 I found that a possible cause for the problem is
the call to printf, which is variadic.

In a musl environment VarArgs are treated like *non* VarArgs.
The handling of this special case was bypassed by the commit 
a4f8551.

The reason is that the arguement `TreatAsVarArg` is only set to `true`
in
an *non* musl env. `TreatAsVarArg` determines the result of
`isVarArg()`.
The `CCIfArgVarArg` class only checks each individual
variable, but not whether `isVarArg()` is true.

Without the special case, the unnamed arguments are always passed
on the stack, as is standard calling convention. But with musl
also unnamed arguments can be passed in registers.

Possibly, this fixes llvm#145206.
…subprogram DIEs (llvm#159104)

With this change, construction of abstract subprogram DIEs is split in
two stages/functions:
creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE)
and its population with children (in
DwarfCompileUnit::constructAbstractSubprogramScopeDIE).
With that, abstract subprograms can be created/referenced from
DwarfDebug::beginModule, which should solve the issue with static local
variables DIE creation of inlined functons with optimized-out
definitions. It fixes llvm#29985.

LexicalScopes class now stores mapping from DISubprograms to their
corresponding llvm::Function's. It is supposed to be built before
processing of each function (so, now LexicalScopes class has a method
for "module initialization" alongside the method for "function
initialization"). It is used by DwarfCompileUnit to determine whether a
DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is
invoked.

DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can
create an abstract or a concrete DIE for a subprogram. It accepts
llvm::Function* argument to determine whether a concrete DIE must be
created.

This is a temporary fix for
llvm#29985. Ideally, it will be
fixed by moving global variables and types emission to
DwarfDebug::endModule (https://reviews.llvm.org/D144007,
https://reviews.llvm.org/D144005).

Some code proposed by Ellis Hoag <[email protected]> in
llvm#90523 was taken for this
commit.
Recognize an MTE tag fault Mach exception. A tag fault is an error
reported by Arm's Memory Tagging Extension (MTE) when a memory access
attempts to use a pointer with a tag that doesn't match the tag stored
with the memory. LLDB will print the tag and address to make the issue
easier to diagnose.

This was hand tested by debugging an MTE enabled binary on an iPhone 17
running iOS 26.

rdar://113575216
…lvm#144744)

Fix `llvm::concat_iterator` for the case of `ValueT` being a pointer to
a common base class to which the result of dereferencing any iterator in
`ItersT` can be casted to.
The change adds two new properties to each `result` object in the SARIF log:

`partialFingerprints`: Contains the "issue hash" that the analyzer already generates for each result, which can help identify a result across runs even if surrounding code changes.

`hostedViewUri`: If running with `-analyzer-format=sarif-html`, this property will now be emitted with the `file:` URL of the generated HTML report for that result.

Along the way, I discovered an existing bug where the HTML diagnostic consumer does not record the path to the generated report if another compilation already created that report. This caused both the SARIF and Plist consumers to be missing the link to the file in all but one of the compilations in case of a warning in a header file. I added a new test to ensure that the generated SARIF for each compilation contains the property.

Finally, I made a few changes to the `normalize_sarif` processing in the tests. I switched to `sed` to allow substitutions. The normalization now removes directory components from `file:` URLs, replaces the `length` property of the source file with a constant `-1`, and puts placeholders in the values of the `version` properties rather than just deleting them. The URL transformation in particular lets us verify that the right filename is generated for each HTML report.

Fixes llvm#158159

rdar://160410408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment