[Lldb-commits] [compiler-rt] [clang] [libc] [llvm] [lldb] [clang-tools-extra] [flang] [mlir] [DAGCombiner] Combine frem into fdiv+ftrunc+fma (PR #67642)
davemgreen wrote: Do you have any analysis on the expected magnitude of the inaccuracy we might expect from performing the fdiv/trunc/fma vs the call to fmod? https://github.com/llvm/llvm-project/pull/67642 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [clang] [flang] [compiler-rt] [llvm] [clang-tools-extra] [libcxxabi] [lldb] [mlir] [openmp] [MachineCopyPropagation] When the source of PreviousCopy is undef, we cannot replace
davemgreen wrote: Hello. I think that if you removed undef from the first instruction the result would still be incorrect. With: ``` $x8 = ORRXrs $xzr, $x0, 0, implicit $w0 $w8 = ORRWrs $wzr, $w0, 0, implicit-def $x8 ``` The second instruction will zero-extend the w0 register to x8. It would be OK to remove the first instruction (it is dead), it is not OK to remove the second if something is relying on the top bits being zero. I assume that's what goes wrong in your case? The top bits are not zero into the function? https://github.com/llvm/llvm-project/pull/74682 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [mlir] [lldb] [openmp] [llvm] [clang] [libcxxabi] [compiler-rt] [clang-tools-extra] [flang] [libcxx] [MachineCopyPropagation] When the source of PreviousCopy is undef, we cannot replace
davemgreen wrote: I don't believe the undef is the issue - I think the issue is that AArch64InstrInfo::isCopyInstrImpl is saying that a W-reg orr is a copy, even if it is really a zextend because the entire X output register is depended upon. Can you try and add something to isCopyInstImpl instead, that says: If the register is virtual then it should not be a subreg, and if the register is physical then there should not be an implicit def of the X reg. Would that solve your issue, and would it cause other problems in AArch64 codegen? Thanks https://github.com/llvm/llvm-project/pull/74682 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [openmp] [clang-tools-extra] [libcxx] [mlir] [clang] [compiler-rt] [lldb] [llvm] [libcxxabi] [flang] [MachineCopyPropagation] When the source of PreviousCopy is undef, we cannot replace
davemgreen wrote: Thanks. It sounds like there are not a lot of code changes, which is a good sign. I didn't expect the debug problems though. I'll try and take a look at the patch. Perhaps you are right that we need a new method for the debug info to use. https://github.com/llvm/llvm-project/pull/74682 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)
davemgreen wrote: IMO This patch looks far too large to sensibly review and needs to be split up. A lot of the changes don't really looks like mechanical renamings, and it is hard to see how they would not break existing uses of llvm arch64 target features? https://github.com/llvm/llvm-project/pull/90320 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)
davemgreen wrote: > This is already split into 18 commits, I don't think there's any reason to > split it into 18 PRs, since comments on one of them likely apply to the > others. I disagree. This is going to be awkward for a lot of users of llvm and contains at least some details I don't agree with. I think it will can cause a lot of subtle bugs and can end up wasting a lot of peoples time. It should at least be awkward for us too. https://github.com/llvm/llvm-project/pull/90320 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)
davemgreen wrote: @tmatheson-arm reached out and we have a bit of a conversation internally. I do think that there is too much going on in this one pr to be sensible to review, but from what I've looked at my main points I think are: - Some AEK names get renamed in ways I would not expect them to, like AEK_FP16 being changed to AEK_FULLFP16. The command-line names or FEAT_ names should probably be what we are aiming for if we are changing them one-way or the other. - Some of the backend features have been renamed in ways that could cause breakages for external users, like +complxnum becoming +fcma. The new name is better, but ideally the old name would continue to work (maybe by adding an alias/extra target feature dependant on the new name? That might not work with negative features). - Some of changes do look mechanical and a good change (P1->p1, V2->v2 etc), and if they were separate they would be easy to get in and out of the way of the contentious stuff that remains. - The ones that have changed should have release notes. https://github.com/llvm/llvm-project/pull/90320 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [AArch64][TargetParser] autogen ArchExtKind enum - renaming (PR #90320)
davemgreen wrote: Rust (https://github.com/rust-lang/rust/blob/79734f1db8dbe322192dea32c0f6b80ab14c4c1d/compiler/rustc_codegen_llvm/src/llvm_util.rs#L229) and zig (https://github.com/ziglang/zig/blob/44db92d1ca90c9cfdfb29fe46f04ff8f11c80901/lib/std/Target/aarch64.zig#L43) are two examples of what I meant by external dependencies. They can adapt, especially if there are release notes, but there may be many more projects out there using the old names and it would be good if they kept working, if we can. https://github.com/llvm/llvm-project/pull/90320 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libc] [clang-tools-extra] [lldb] [flang] [mlir] [llvm] [clang] [compiler-rt] [libcxx] [AArch64] add intrinsic to generate a bfi instruction (PR #79672)
davemgreen wrote: Hello. Can you explain why this is needed, as opposed to using the equivalent shift/and/ors? https://github.com/llvm/llvm-project/pull/79672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [compiler-rt] [libc] [flang] [mlir] [libcxx] [lldb] [llvm] [clang] [AArch64] add intrinsic to generate a bfi instruction (PR #79672)
davemgreen wrote: OK. We would not usually add intrinsics like this without a strong motivating case, that could not be optimized in some other way. It is better to use target independent options when available, and inline assembly is available as a fallback if it is really needed. But I would recommend that they use normal and/or/shift operations and let us know about places the compiler isn't optimizing them as well as it could be. https://github.com/llvm/llvm-project/pull/79672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [flang] [compiler-rt] [clang] [llvm] [mlir] [clang-tools-extra] [lldb] [libc] [libcxx] [AArch64] add intrinsic to generate a bfi instruction (PR #79672)
davemgreen wrote: I see. The issue is that the opposite is often true as well - if we add a target specific intrinsic for this then, whilst we get a single instruction being emitted, we don't see all the other optimizations that the compiler can and should be performing. Things like constant folding, combining into other instructions, known-bits analysis or any form of vectorization will all be blocked by the intrinsic. It can take quite some work to add all those features in (if they are possible), and without them can potentially lead to worse results. Plus more things to maintain. BFI isn't a trivial instructions to match as it involves certain masks and shifts. There might certainly be advantages to having an intrinsic. I would like to try and see what the problems would be with generated code using normal operations first though, if we can. If there are optimizations we can make based on the existing code then that would help in all cases (c, mlir, rust, etc), not just frontends that are producing the intrinsics. https://github.com/llvm/llvm-project/pull/79672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [APInt] Fix APInt constructions where value does not fit bitwidth (NFCI) (PR #80309)
@@ -1159,7 +1159,9 @@ class ARMOperand : public MCParsedAsmOperand {
if (!isImm()) return false;
const MCConstantExpr *CE = dyn_cast(getImm());
if (!CE) return false;
davemgreen wrote:
Can this check that CE->getActiveBits() > 32?
https://github.com/llvm/llvm-project/pull/80309
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
