[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

efriedma updated this revision to Diff 557651. efriedma added a comment. Fix the calling convention for functions returning C++ classes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157547/new/ https://reviews.llvm.org/D157547 Files: clang/lib/

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

efriedma added a comment. > For assembly, I'm not really comfortable with the fix you're proposing; it > relies on the order in which functions are defined in assembly. I think LLVM > usually ends up emitting module-level inline assembly before generated code, > but it seems fragile to depend o

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

efriedma added a comment. Given the complexity here, I agree this is probably the best we can reasonably do. Code and test changes LGTM. That said, this is missing a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86310/new/ https://

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

efriedma added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:14254 int SectionFlags = ASTContext::PSF_Read; -if (var->getType().isConstQualified()) { - if (HasConstInit) rnk wrote: > I think this is not compatible with MSVC. MSVC uses sim

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. In D76096#4540442 , @nickdesaulniers wrote: > Consider the following code: > > long x [] = {1, 2, 3, 4, 5 + 4}; > > Even with some of my recent c

[PATCH] D156378: [clang][CGExprConstant] handle unary negation on integrals

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1366 + llvm::Constant *VisitUnaryOperator(UnaryOperator *U, QualType T) { +switch (U->getOpcode()) { StmtVisitor supports defining a function "VisitUnaryMinus", so you don't h

[PATCH] D156466: [clang][CGExprConstant] handle implicit widening/narrowing Int-to-Int casts

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1152 + return CI; +llvm::APInt A = CI->getValue().sextOrTrunc(DstWidth); +return llvm::ConstantInt::get(CGM.getLLVMContext(), A); sextOrTrunc() i

[PATCH] D156482: [clang][CGExprConstant] handle FunctionToPointerDecay

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1128 +case CK_NoOp: +case CK_NonAtomicToAtomic: return Visit(subExpr, destType); I think I'd prefer to continue treating an undecayed function as an "lvalue", to keep

[PATCH] D155850: [Clang][CodeGen][RFC] Add codegen support for C++ Parallel Algorithm Offload

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5559 +return EmitStdParUnsupportedBuiltin(this, FD); + else +ErrorUnsupported(E, "builtin function"); Else-after-return. Comment at: clang/lib/CodeGen/CodeGenM

[PATCH] D156172: [clang][CodeGen] Emit annotations for function declarations.

efriedma added a comment. I'm happy with this approach. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4327 +if (D && D->hasAttr() && isa(Entry)) + DeferredAnnotations[cast(Entry)] = cast(D); + This doesn't quite seem sufficient... I'd expect you'd wa

[PATCH] D156378: [clang][CGExprConstant] handle unary negation on integrals

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156378/new/ https://reviews.llvm.org/D156378 ___

[PATCH] D156466: [clang][CGExprConstant] handle implicit widening/narrowing Int-to-Int casts

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156466/new/ https://reviews.llvm.org/D156466 ___

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

efriedma added a comment. I see what you're getting at here... but I don't think this works quite right. If the empty class has a non-trivial constructor, we have to pass the correct "this" address to that constructor. Usually a constructor for an empty class won't do anything with that addre

[PATCH] D155850: [Clang][CodeGen][RFC] Add codegen support for C++ Parallel Algorithm Offload

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM (but please don't merge until we reach consensus on the overall feature) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155850/new/ https://reviews.llvm.org/D155850 _

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

efriedma added a comment. The overall approach here seems reasonable. I mean, technically the undefined behavior is happening in the library, but detecting it early seems like a good idea. This approach does have a significant limitation, though: CGBuiltin won't detect cases that involve taki

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

efriedma created this revision. efriedma added reviewers: dpaoliello, mstorsjo, bcl5980, jacek. Herald added subscribers: zzheng, hiraditya, kristof.beyls. Herald added a project: All. efriedma requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-com

[PATCH] D156726: Make globals with mutable members non-constant, even in custom sections

efriedma added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:14254 int SectionFlags = ASTContext::PSF_Read; -if (var->getType().isConstQualified()) { - if (HasConstInit) dblaikie wrote: > rnk wrote: > > rsmith wrote: > > > efriedma wrote:

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

efriedma added a comment. I don't really like `extern cl::opt PrintPipelinePasses;` (as opposed to implementing a proper driver option that calls a proper API); secret handshakes like this make it harder for people to write non-clang frontends. But I won't block the patch just for that, I gues

[PATCH] D154270: [clang][CodeGen] Fix global variables initialized with an inheriting constructor.

efriedma created this revision. efriedma added reviewers: rsmith, rnk, rjmccall, akhuang. Herald added a subscriber: kristof.beyls. Herald added a project: All. efriedma requested review of this revision. Herald added a project: clang. For inheriting constructors which have to be emitted inline, w

[PATCH] D154270: [clang][CodeGen] Fix global variables initialized with an inheriting constructor.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:2459 + if (!isa(Arg.getAnyValue())) +Arg.getAnyValue()->setName(D.getName()); rsmith wrote: > Is it feasible to only set the name if the value doesn't already have a name, > or do we

[PATCH] D154284: [C11] Correct global atomic pointer initialization from an integer constant

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. The testcase doesn't actually trigger the assertion... but I guess it still tests the output, so that's probably okay? Maybe add a case `static _Atomic(int *) glob_pointer_from_long = 0LL

[PATCH] D154270: [clang][CodeGen] Fix global variables initialized with an inheriting constructor.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb4bae3fd8ede: [clang][CodeGen] Fix global variables initialized with an inheriting… (authored by efriedma). Repository: rG LLVM Github Monorepo C

[PATCH] D153835: [Sema] Clone VisibilityAttr for functions in template instantiations

efriedma added a comment. Can you write the complete rule we're trying to follow here? Like, if you have a free function template, prioritize attributes in the following order ..., if you have a member function, use the following order ..., etc. I know that's a significant amount of writing,

[PATCH] D154911: Enabling fstack_clash_protection for arm32 bit, thumb and thumb2 mode

efriedma added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3460 + } else if (EffectiveTriple.isArm() || EffectiveTriple.isThumb()) { +CmdArgs.push_back("-mstack-probe-size=1024"); + } Why 1024? Comment at: llvm/lib/

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

efriedma added a comment. If I follow correctly, this is basically undoing the splitting that was done by the command that produced the bitcode file? I guess that could be useful. But it requires either renaming your object files from the default ".o" to ".bc", or explicitly passing "-x ir"?

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

efriedma added a comment. > @efriedma would you consider the changes suggested by @hvdijk sufficient > under any circumstances or would you still insist on fully compatible > AutoUpgrade, given the above discussion? If the requirement is "we can mix old and new IR", we have to do it correctly,

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

efriedma added a comment. > The only problem that approach 2 solves is to ensure that a non-clang > frontend using i128 https://reviews.llvm.org/D86310#2231136 has an example where IR generated by clang breaks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86310/new/ https://reviews.

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

efriedma added a comment. Do we need CodeGen testcases here for full coverage? The testcases from the issue passed with -fsyntax-only. With this patch, the following cases produce errors that don't really make sense to me: consteval int f(int x) { return x; } struct SS {

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

efriedma added a comment. In D86310#4498721 , @hvdijk wrote: > In D86310#4498575 , @efriedma wrote: > >> https://reviews.llvm.org/D86310#2231136 has an example where IR generated by >> clang breaks. > > clang bases

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

efriedma added a comment. I'm not personally involved with any workflows that care about autoupgrade, so I'm not really invested in ensuring it's stable. If everyone agrees the impact is small enough that we're willing to just break autoupgrade to the extent it's relevant, I'll withdraw my obj

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

efriedma added a comment. > You have something specific in mind? I was thinking something along the lines of the original testcase in 63742, which successfully compiled in 16, started producing an assertion on main, and successfully compiles with this patch. Repository: rG LLVM Github Monor

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

efriedma added a comment. Test changes look fine. (I'm not really comfortable reviewing the Sema logic.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new/ https://reviews.llvm.org/D155175 ___ cf

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

efriedma updated this revision to Diff 552917. efriedma added a comment. Update to address issues found so far. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157547/new/ https://reviews.llvm.org/D157547 Files: clang/lib/CodeGen/CGCXX.cpp llvm/

[PATCH] D158857: [clang][aarch64] Add support for the MS qualifiers __ptr32, __ptr64, __sptr, __uptr for aarch64

efriedma added a comment. If you don't have autoupgrade, that means you can't link IR built with older versions of LLVM to IR built with newer versions. llvm::UpgradeDataLayoutString is designed to fix this incompatibility. It's probably worth doing here given that it's relatively straightfor

[PATCH] D154869: [Flang] [FlangRT] Introduce FlangRT project as solution to Flang's runtime LLVM integration

efriedma added a comment. Maybe split the changes to reformat the unittests into a separate patch? Otherwise, I'm happy with the current state of this patch, but probably someone more active in flang should approve. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D156172: [clang][CodeGen] Emit annotations for function declarations.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156172/new/ https://reviews.llvm.org/D156172 ___

[PATCH] D138511: [CodeGen][AArch64] Fix AArch64ABIInfo::EmitAAPCSVAArg crash with empty record type in variadic arg

efriedma added a comment. It looks like AArch64ABIInfo::classifyArgumentType does a slightly more complicated check for "empty"; does that matter here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138511/new/ https://reviews.llvm.org/D138511 ___

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

efriedma added a comment. That's roughly what I was thinking, yes. The one missing piece is the code to modify the constructor priorities to make sure that "constant" variables get initialized first. (This ensures we honor the C++ rules for "constant initialization".) Comme

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4822 Init = Initializer; + if (Initializer->isDLLImportDependent()) +NeedsGlobalCtor = true; This check probably needs to be inside tryEmitForInitializer, so other

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

efriedma added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:13896 + Diag(var->getLocation(), diag::err_constexpr_var_requires_const_init) + << var << Init->getSourceRange(); + } zahiraam wrote: > efriedma wrote: > > I don't

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

efriedma added a comment. Looking at this again, I just thought of something: in C mode, we probably don't want to generate global constructors, so we might want to continue to print an error in that case. Comment at: clang/lib/Sema/SemaDecl.cpp:13896 + Diag(var->get

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

efriedma added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:13896 + Diag(var->getLocation(), diag::err_constexpr_var_requires_const_init) + << var << Init->getSourceRange(); + } rnk wrote: > zahiraam wrote: > > efriedma wrot

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

efriedma added a reviewer: aeubanks. efriedma added a subscriber: aeubanks. efriedma added a comment. I'm not really familiar with the way constructor priorities work on Windows works... see https://reviews.llvm.org/D131910 ? Adding @aeubanks as a reviewer. CHANGES SINCE LAST ACTION https://

[PATCH] D139167: [clang][Windows]Ignore Options '/d1nodatetime' and '/d1import_no_registry'

efriedma added a subscriber: steplong. efriedma added a comment. FYI, there's some discussion of /d1nodatetime floating around, even though it isn't formally documented: it disables the definition of the `__DATE__`, `__TIME__`, and `__TIMESTAMP__` macros. (No idea about d1import_no_registry, th

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma marked 2 inline comments as done. efriedma added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1059 +llvm::Type *ElemTy = ArrayTy->getElementType(); +bool ZeroInitializer = constant->isNullValue(); llvm::Constant *OpValue, *PaddedOp;

[PATCH] D76504: [clang] Fix crash during template sema checking

efriedma added a comment. Please post patches against master, not previous versions of the patch. I think you want to check isValueDependent(); isInstantiationDependent() includes some other stuff that isn't relevant here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D76527: Don't export symbols from clang/opt/llc if plugins are disabled.

efriedma created this revision. efriedma added reviewers: MaskRay, serge-sans-paille, Meinersbur. Herald added subscribers: cfe-commits, dexonsmith, mgorny. Herald added a project: clang. The only reason we export symbols from these tools is to support plugins; if we don't have plugins, exporting

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma created this revision. efriedma added reviewers: glider, jfb. Herald added a subscriber: dexonsmith. Herald added a project: clang. The code was pretending to be doing something useful with vectors, but really it was doing nothing: the element type of a vector is always a scalar type, so

[PATCH] D76504: [clang] Fix crash during template sema checking

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Sema/SemaChecking.cpp:1655 +// greater than 0. When `size` is value dependent we cannot evaluate its +// value so we bail<< out. +i

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

efriedma added a comment. I think we should remove the LangRef rule that says "sret" pointers have to be dereferenceable/naturally aligned, and let the frontend add explicit aligned/dereferenceable markings where appropriate. (At that point, sret has no target-independent meaning; it's just to

[PATCH] D76527: Don't export symbols from clang/opt/llc if plugins are disabled.

This revision was automatically updated to reflect the committed changes. Closed by commit rG896335bfb8ea: Don't export symbols from clang/opt/llc if plugins are disabled. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76527/ne

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma added a comment. Not sure what sort of test would catch this; it's not crashing, and the types with the tail padding issue are not naturally exposed by target intrinsic headers (so it would only show up for code using ext_vector_type, or maybe OpenCL code). I only tripped over the iss

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma updated this revision to Diff 252169. efriedma edited the summary of this revision. efriedma added a comment. Add testcase to show missing vector handling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76528/new/ https://reviews.llvm.org/D

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma added a comment. Actually looking again, I'm not sure there's any way to make clang generate a bool vector at the moment. Added tests for the rest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76528/new/ https://reviews.llvm.org/D76528

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:233 + bool Changed = false; + for (auto *F : Functions) { +DominatorTree *DT = &getAnalysis(*F).getDomTree(); Iterating over a SmallPtrSet is non-deterministic. In th

[PATCH] D76689: [Sema][SVE] Fix handling of initialisers for built-in SVE types

efriedma added a comment. Semantically, this makes sense. I'm not really happy with the use of the term "scalar initializer" to refer to initializing a vector. Seems likely to confuse users. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76689/new

[PATCH] D76694: [Sema][SVE] Allow casting SVE types to themselves in C

efriedma added a comment. Is it not legal to cast an SVE type to any type other than itself? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76694/new/ https://reviews.llvm.org/D76694 ___ cfe-commits mai

[PATCH] D76690: [AST][SVE] Treat built-in SVE types as POD

efriedma added inline comments. Comment at: clang/lib/AST/Type.cpp:2515 + if (BaseTy->isSizelessBuiltinType()) +return true; + Can you rearrange this so isSizelessBuiltinType() is at the bottom? It should be rare. (Assuming it doesn't need to be before th

[PATCH] D76693: [Sema][SVE] Allow ?: to select between SVE types in C

efriedma added a comment. Do we want to allow stuff like `x ? (svint8_t)0 : (signed char)0` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76693/new/ https://reviews.llvm.org/D76693 ___ cfe-commits mail

[PATCH] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7571 + else if (Builtin->LLVMIntrinsic != 0) { +llvm::Type* OverloadedTy = getSVEType(TypeFlags); + I'm not sure why you need a second way to convert SVE types separate from Convert

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma updated this revision to Diff 252432. efriedma edited the summary of this revision. efriedma added a comment. Rebased. (CGDecl.cpp changes landed separately.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.or

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

This revision was automatically updated to reflect the committed changes. Closed by commit rG3f1defa6e2df: [clang codegen] Clean up handling of vectors with trivial-auto-var-init. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D76694: [Sema][SVE] Allow casting SVE types to themselves in C

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Okay, then LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76694/new/ https://reviews.llvm.org/D76694 ___

[PATCH] D76693: [Sema][SVE] Allow ?: to select between SVE types in C

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76693/new/ https://reviews.llvm.org/D76693 ___

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

efriedma added a comment. If it's just tramp3d-v4, I'm not that concerned... but that's a weird result. On x86 in particular, alignment markings have almost no effect on optimization, generally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74183

[PATCH] D76692: [AST][SVE] Treat built-in SVE types as trivial

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76692/new/ https://reviews.llvm.org/D76692 __

[PATCH] D76691: [AST][SVE] Treat built-in SVE types as trivially copyable

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76691/new/ https://reviews.llvm.org/D76691 ___

[PATCH] D76690: [AST][SVE] Treat built-in SVE types as POD

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Okay, then LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76690/new/ https://reviews.llvm.org/D76690

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma updated this revision to Diff 252645. efriedma added a comment. Address lint comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.org/D75661 Files: clang/lib/CodeGen/CGExprConstant.cpp llvm/include/ll

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma updated this revision to Diff 252659. efriedma added a comment. Herald added subscribers: Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar, rriddle, mehdi_amini. Fix bug in GlobalOpt. Add fixes for MLIR. Repository

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

efriedma added subscribers: hfinkel, jdoerfert. efriedma added a comment. That makes sense. I would slightly lean towards not generating the assumptions, given the current state of assumptions and the likely benefit in this context. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

efriedma created this revision. efriedma added a reviewer: rsmith. Herald added a project: clang. -Wthread-safety was failing to detect certain AST patterns it should detect. Make the pattern detection a bit more comprehensive. Due to an unrelated bug involving template instantiation, this showe

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

efriedma updated this revision to Diff 253253. efriedma added a comment. Add support for conversion operators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76943/new/ https://reviews.llvm.org/D76943 Files: clang/lib/Analysis/ThreadSafety.cpp

[PATCH] D77056: RFC: [Sema][SVE] Allow non-member operators for SVE types

efriedma added a comment. I'm concerned we're going to run into trouble if two people define different SVE "libraries", and you try to link both of them into the same program. If you're not careful, you end up with ODR violations. The scope of this is sort of restricted in normal C++: class a

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

This revision was automatically updated to reflect the committed changes. Closed by commit rG24485aec4750: [clang analysis] Make mutex guard detection more reliable. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D76943?vs=253253&id=253657#toc Repository: rG LLVM G

[PATCH] D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract

efriedma added a comment. I can understand why you might want the new intrinsics as a temporary measure, but I don't see the point of removing the already working support for llvm.sadd. etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77054/new/

[PATCH] D77056: RFC: [Sema][SVE] Allow non-member operators for SVE types

efriedma added a comment. > One option would to be wait until there's at least one type that > isSizelessBuiltinType but isn't an SVE type. Another would be to introduce it > when a feature seems too SVE-specific Probably not a big deal either way, as long as we document the intent in the code

[PATCH] D77048: [Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:613 + + // Add padding bits in case of over-sized bit-field. + // "The first sizeof(T)*8 bits are used to hold the value of the bit-field, The existing code in ConstantAggregat

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

efriedma added a comment. I'd prefer not to revert a change that's three months old without some sort of evidence the issue is actually the compiler's fault. If nobody else has seen an issue, it's probably okay to let it sit for a day or two. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D72467: Remove "mask" operand from shufflevector.

efriedma added a comment. (Also merged followup https://github.com/llvm/llvm-project/commit/ba4764c2cc14b0b495af539a913de10cf8268420 to fix a memory leak caught by the bots.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72467/new/ https://review

[PATCH] D77048: [Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77048/new/ https://reviews.llvm.org/D77048 ___

[PATCH] D77131: [clang] Move branch-protection from CodeGenOptions to LangOptions

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77131/new/ https://reviews.llvm.org/D77131 ___ cfe-commits mailing list cfe-commi

[PATCH] D77236: [SVE] Cleanup prior to introdution of FixedVectorType

efriedma added a comment. My thoughts, in no particular order: 1. This is orthogonal to splitting VectorType, as far as I can tell. `Ty->getVectorNumElements()` works equally well whether the implementation asserts it's a VectorType that isn't scalable, or asserts it's a FixedVectorType. 2. T

[PATCH] D77313: [AST] Allow VectorTypes of 1-256 elements, and powers of two up to 2**31.

efriedma added a comment. I think I would rather just pay the extra 8 bytes per VectorType, and expand this to support all vector types supported by LLVM. It's not like we allocate enough VectorTypes for it to matter, anyway. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract

efriedma added a comment. You should be able to refactor the patterns into the definitions of the multiclasses sve_int_bin_cons_arit_0 and sve_int_arith_imm0, to avoid repeating them four times. (You might want to look at other places using null_frag in SVEInstrFormats.td for inspiration.) C

[PATCH] D77335: [AST] clang::VectorType supports any size (that fits in unsigned)

efriedma added a comment. If you're going to add code to check for it, we might as well add testcases for ridiculous sizes, like `(__int128_t)1 << 100`. I think this makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77335/new/ https://re

[PATCH] D77335: [AST] clang::VectorType supports any size (that fits in unsigned)

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77335/new/ https://reviews.llvm.org/D77335 ___

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

efriedma added a comment. I'm not really a big fan of running tests with the host target triple, anyway; it seems to create work with almost no benefit. I'd be happy to just run the test with one target that has native atomics, and one target that doesn't. (The relevant code is all target-ind

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

efriedma added a comment. On a side-note, we could enhance the test runner to support "XFAIL: riscv32-default-target" if we thought it would be useful. But again, I'm not a fan of tests that depend on the default target in the first place. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D74812: [Sema] Teach -Warm-interrupt-safety about func ptrs

efriedma added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:5931 + if (Caller->hasAttr()) { +const Decl *CalleeDecl = FDecl; +if (const auto *UO = dyn_cast(Fn->IgnoreParens())) { jroelofs wrote: > This feels very fragile, and I kn

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

efriedma added a comment. Yes, makes sense Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74847/new/ https://reviews.llvm.org/D74847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

efriedma added a comment. Where are you planning to use this? Comment at: clang/lib/Sema/SemaStmt.cpp:2817 + + unsigned targetCacheLineSize = Ctx.getTargetInfo().getCPUCacheLineSize(); + if (!targetCacheLineSize) "64" here is arbitrary; it's not actually supp

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

efriedma added a comment. I'm not sure I like the wording; it's a complicated sentence to understand. Can we make it sort of list-like? "for each memory location accessed through a noalias pointer, either: 1. the location is not modified during the execution of the function. 2. all accesses t

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74847/new/ https://reviews.llvm.org/D74847 ___

[PATCH] D74912: [AArch64][SVE] Add SVE2 intrinsics for bit permutation & table lookup

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1220 +void AArch64DAGToDAGISel::SelectTableSVE2(SDNode *N, unsigned Opc) { + SDLoc DL(N); Is it possible to write this as a TableGen pattern? We manage for other vari

[PATCH] D62962: Clang implementation of sizeless types

efriedma added a comment. Can you make a list of the complete stack of patches here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62962/new/ https://reviews.llvm.org/D62962 ___ cfe-commits mailing lis

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

efriedma added a comment. In D74935#1886020 , @nhaehnle wrote: > I find this phrasing pretty confusing. How about the following: > > > This indicates that objects accessed via pointer values based on the > > argument or return value are not **modified**,

[PATCH] D66839: Fix stack address builtin for negative numbers

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 ___

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Current language seems fine. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74935/new/ https://reviews.llvm.org/D74935 _

[PATCH] D74912: [AArch64][SVE] Add SVE2 intrinsics for bit permutation & table lookup

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74912/new/ https://reviews.llvm.org/D74912 ___ cfe-commits mailing list cfe-commi

<    8   9   10   11   12   13   14   15   16   17   >