mattdr added a comment.
Took a quick look and seems sane -- will look after Craig's comment is
addressed and build is passing
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79910/new/
https://reviews.llvm.org/D79910
_
mattdr added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:248-249
+ def warn_slh_does_not_support_gcc_asm_goto : Warning<
+"speculative load hardening does not support use of GCC asm goto. asm goto
"
+"detected with SLH">, InGroup>
mattdr added a comment.
Not sure we want to move this into the Parser -- SLH is a property of code
generation, and I think it's possible (through LTO?) to mismatch the flags
between parse and codegen.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D
mattdr added a comment.
I'm no expert in the pass manager, but given the very targeted applicability of
LVI it definitely seems like our goal should be 0% impact for the vast majority
of compilations that don't concern themselves with it.
Is there a way to require the pass be enabled for the co
mattdr added subscribers: rsmith, mattdr.
mattdr added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.def:369
+BENIGN_LANGOPT(SpeculativeLoadHardeningEnabled, 1, 0,
+ "Speculative load hardening enabled")
I've read the descript
mattdr accepted this revision.
mattdr marked 2 inline comments as done.
mattdr added a comment.
The extra comments and the new variable name are all helpful. Thanks again.
Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:321-322
+ for (auto UseID
mattdr added a comment.
Calling special attention to the comment at line 341, since I think it affects
the correctness of the pass.
Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:287
+ // The `Transmitters` map memoizes transmitters found for each def.
mattdr marked an inline comment as done.
mattdr added inline comments.
Comment at: llvm/lib/Target/X86/ImmutableGraph.h:366
+std::vector TrimmedNodes(TrimNodes.size());
+for (size_type I = 0; I < TrimNodes.size(); ++I) {
+ TrimmedNodes[I] = TrimmedNodesSoFar;
---
mattdr accepted this revision.
mattdr added a comment.
This revision is now accepted and ready to land.
Accepting modulo some comments to be addressed. Most of my review effort was
spent on the data structure and algorithm employed as well as code style and
readability.
I am least confident abo
mattdr added a comment.
I accidentally spent time reviewing this, only to cross-reference something in
the LLVM code and find another diff (https://reviews.llvm.org/D76812) was
written to answer Zola's request and has already been submitted.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org
mattdr added a comment.
@craig.topper can you please update/rebase this stack to remove the early LVI
CFI work that ended up landed in https://reviews.llvm.org/D76812 instead?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75936/new/
https://reviews.llvm.org/D75936
__
mattdr added a comment.
I'll wait for current comments to be addressed before doing my next round here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75936/new/
https://reviews.llvm.org/D75936
___
cfe-commits mailing list
cfe-commits@lists.
mattdr added a comment.
Some more comments. FWIW, I'm doing rounds of review as I can in some evening
quiet or during my son's nap. This is a huge change and it's really hard to get
any part of it into my head at once in a reasonable amount of time.
Comment at: llvm/lib/Targe
mattdr added inline comments.
Comment at: llvm/lib/Target/X86/ImmutableGraph.h:318
+}
+auto *VertexArray = new Node[VertexSize + 1 /* terminator node */];
+auto *EdgeArray = new Edge[EdgeSize];
sconstab wrote:
> mattdr wrote:
> > sconstab wrote:
> > >
mattdr added inline comments.
Comment at: llvm/lib/Target/X86/ImmutableGraph.h:318
+}
+auto *VertexArray = new Node[VertexSize + 1 /* terminator node */];
+auto *EdgeArray = new Edge[EdgeSize];
sconstab wrote:
> mattdr wrote:
> > As a general rule `ne
mattdr added a comment.
Adding a few early style notes for the next round, but overall echo @chandlerc
that this seems significantly outside of normal LLVM code.
Comment at: llvm/lib/Target/X86/ImmutableGraph.h:26
+
+#ifndef IMMUTABLEGRAPH_H
+#define IMMUTABLEGRAPH_H
-
16 matches
Mail list logo