[PATCH] D158883: [Matrix] Try to emit fmuladd for both vector and matrix types

2023-09-06 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D158883#4638648 , @thegameg wrote: > In D158883#4635997 , @uweigand > wrote: > >> The newly added test cases in ffp-model.c fail on SystemZ, making CI red: > > Should be fixed,

[PATCH] D158883: [Matrix] Try to emit fmuladd for both vector and matrix types

2023-09-02 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. The newly added test cases in ffp-model.c fail on SystemZ, making CI red: https://lab.llvm.org/buildbot/#/builders/94/builds/16280 The root cause seems to be that by default, the SystemZ back-end targets a machine without SIMD support, and therefore vector return types

[PATCH] D149548: [IR] Update to use new shufflevector semantics

2023-06-12 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D149548#4413639 , @nlopes wrote: > If a vector is fully initialized with `insertvector` (i.e., one operation per > index), then the value of the base vector is irrelevant. It can be poison. > Poison in vectors is

[PATCH] D149548: [IR] Update to use new shufflevector semantics

2023-06-12 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. So the semantics of the `vec_promote(a, b)` intrinsic is defined as: > Returns a vector with a in element position b. The result is a vector with a > in element position b. [...] The other elements of the vector are undefined. This is currently implemented by using

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-12 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139444/new/ https://reviews.llvm.org/D139444 ___ cfe-commits mailing list

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-09 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D139444#3983679 , @uweigand wrote: > In D139444#3982205 , @probinson > wrote: > >> If you can tell me the `platform.system()` value to look for to detect z/OS, >> I can do that. > >

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-09 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D139444#3982205 , @probinson wrote: > If you can tell me the `platform.system()` value to look for to detect z/OS, > I can do that. I don't know - this value is defined by the Python implementation on z/OS. On most

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-07 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D139444#3975182 , @probinson wrote: > The changes in this patch assume that there aren't any possible suffixes > after the `-zos` part of the triple (no version numbers, like you might find > with darwin or macos, and

[PATCH] D136145: [IR][RFC] Restrict read only when cache type of llvm.prefetch is instruction

2022-10-18 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: llvm/test/CodeGen/SystemZ/prefetch-01.ll:18 -; Check that instruction write prefetches are ignored. -define dso_local void @f2(ptr %ptr) { -; CHECK-LABEL: f2: -; CHECK-NOT: %r2 -; CHECK: br %r14 - call void @llvm.prefetch(ptr %ptr,

[PATCH] D136040: [X86] Support PREFETCHI instructions

2022-10-17 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D136040#3862386 , @pengfei wrote: > Sure, it is possible. But at least for now, there's no real target requires > it. Checked with `grep -rwn 'llvm.prefetch.*i32 0\s*)' llvm/test/CodeGen/`. But that's just within the LLVM

[PATCH] D136040: [X86] Support PREFETCHI instructions

2022-10-17 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D136040#3862225 , @pengfei wrote: > 3. Add semacheck for prefetch write to instruction cache; > > I think the affected ARM and SystemZ tests are not valid before. Could > @t.p.northover and @uweigand help to have a look?

[PATCH] D133338: [clang][PowerPC] PPC64 VAArg use coerced integer type for direct aggregate fits in register

2022-09-13 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. This LGTM now, but maybe some of the PowerPC reviewers would like to comment as well. @nemanjai ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D18/new/ https://reviews.llvm.org/D18

[PATCH] D133338: [clang][PowerPC] PPC64 VAArg use coerced integer type for direct aggregate fits in register

2022-09-08 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:5471 +if (CoerceTy->isIntegerTy() && CoerceTy->getIntegerBitWidth() < GPRBits) + ForceRightAdjust = true; + } Are all these checks really necessary here? This seems

[PATCH] D133338: [clang][PowerPC] PPC64 VAArg use coerced integer type for direct aggregate fits in register

2022-09-07 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. I think it is correct to implement this in Clang. Note that on SystemZ (another big-endian platform), we also implement this in `EmitVAArg`. Of course the details are different since we're not using `emitVoidPtrVAArg` on that platform. However, I'm not sure if

[PATCH] D129562: [SystemZ] Enable `-mtune=` option in clang.

2022-07-13 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129562/new/ https://reviews.llvm.org/D129562

[PATCH] D127498: [SystemZ/z/OS] Set DWARF version to 4 for z/OS.

2022-06-10 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127498/new/ https://reviews.llvm.org/D127498 ___ cfe-commits mailing list

[PATCH] D123627: Correctly diagnose prototype redeclaration errors in C

2022-04-14 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D123627#3451373 , @aaron.ballman wrote: > Thank you for letting me know -- I've speculatively fixed the issue in > 726901d06aab2f92d684d28507711308368c29d6 >

[PATCH] D123627: Correctly diagnose prototype redeclaration errors in C

2022-04-14 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. It looks like this caused the LNT build bot to fail (at least on s390x), because one of the test cases now triggers this error: https://lab.llvm.org/buildbot/#/builders/45/builds/6787 /usr/bin/make -f

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-21 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D109362#3013253 , @anirudhp wrote: > - Reduced the number of test lines in target-data.c, since we don't have to > check for every combination of arch,cpu for the SystemZ target (for both elf > and z/OS) These changes

[PATCH] D91944: OpenMP 5.0 metadirective

2021-09-20 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D91944#3009868 , @cchen wrote: > The SystemZ issue is due to the fact that we assumed that `device(cpu)` > should be evaluated to true and `device(gpu)` should be evaluated to false in > the test so the test should be fixed

[PATCH] D91944: OpenMP 5.0 metadirective

2021-09-20 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Looks like this was committed again, breaking the SystemZ build bots once again: https://lab.llvm.org/buildbot/#/builders/94/builds/5661 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-15 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This LGTM now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109362/new/ https://reviews.llvm.org/D109362 ___ cfe-commits mailing list

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-15 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: llvm/docs/LangRef.rst:2596 * ``e``: ELF mangling: Private symbols get a ``.L`` prefix. +* ``l``: GOFF mangling: Private symbols get a ``.L`` prefix. * ``m``: Mips mangling: Private symbols get a ``$`` prefix.

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-15 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D109362#3000284 , @anirudhp wrote: > In D109362#2999688 , @uweigand > wrote: > >> Looking at the common code parts, it seems the behavior of MM_GOFF is >> actually identical to

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-14 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Looking at the common code parts, it seems the behavior of MM_GOFF is actually identical to MM_ELF. Is this correct? If so, do we really need a different format type here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout

2021-09-07 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. SystemZ specific parts LGTM, but it would be good to have someone else review the common code / IR changes as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109362/new/ https://reviews.llvm.org/D109362

[PATCH] D105629: [TSan] Add SystemZ support

2021-07-14 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. The SystemZ specific changes all look good to me now, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105629/new/ https://reviews.llvm.org/D105629 ___ cfe-commits mailing

[PATCH] D105629: [TSan] Add SystemZ support

2021-07-13 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123 +#if defined(__s390x__) + ProtectRange(HiAppMemEnd(), 0xf000ull); +#endif iii wrote: > uweigand wrote: > > iii wrote: > > > uweigand wrote: > > > > Did

[PATCH] D105629: [TSan] Add SystemZ support

2021-07-13 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123 +#if defined(__s390x__) + ProtectRange(HiAppMemEnd(), 0xf000ull); +#endif iii wrote: > uweigand wrote: > > Did you test this on older kernels without

[PATCH] D105629: [TSan] Add SystemZ support

2021-07-13 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. See inline comments ... otherwise the SystemZ platform-specific parts look good to me. Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp:123 +#if defined(__s390x__) + ProtectRange(HiAppMemEnd(), 0xf000ull); +#endif

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Yes, this patch fixes the problem for me. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96033/new/ https://reviews.llvm.org/D96033 ___ cfe-commits mailing list

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-18 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D96033#2765954 , @v.g.vassilev wrote: > @hubert.reinterpretcast, thanks for the feedback. I have created a patch as > discussed -- https://reviews.llvm.org/D102688 > > @uweigand, thanks for reaching out. I believe the patch

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-18 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Looks like this is also failing on s390x: error: Added modules have incompatible data layouts: E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64 (module) vs E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64 (jit) The problem here is that on s390x we use a

[PATCH] D102064: Parse vector bool when stdbool.h and altivec.h are included

2021-05-11 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D102064#2751089 , @ZarkoCA wrote: > In D102064#2751001 , @uweigand > wrote: > >> This means the implementation now deviates from the documented vector >> extension syntax, right?

[PATCH] D102064: Parse vector bool when stdbool.h and altivec.h are included

2021-05-11 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. This means the implementation now deviates from the documented vector extension syntax, right? I guess it's strictly an extension of the documented syntax, but that may still lead to incompatibilities with other compilers for the platform. If we want to make such a

[PATCH] D97901: [SystemZ] Test for infinity in testFPKind().

2021-03-15 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97901/new/ https://reviews.llvm.org/D97901 ___ cfe-commits mailing list

[PATCH] D97901: [SystemZ] Test for infinity in testFPKind().

2021-03-04 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:7229 + case Builtin::BI__builtin_isfinite: +Invert = true; +LLVM_FALLTHROUGH; thopre wrote: > jonpa wrote: > > What are these variants all about...? > > > They

[PATCH] D96568: [CFE, SystemZ] Emit s390.tdc instrincic for __builtin_isnan in Constrained FP mode.

2021-02-18 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM as well, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96568/new/ https://reviews.llvm.org/D96568 ___ cfe-commits

[PATCH] D82862: [ThinLTO] Always parse module level inline asm with At dialect

2021-01-26 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D82862#2513044 , @rnk wrote: > In D82862#2512908 , @uweigand wrote: > >> So why do you want GNU inline asm for clang-cl anyway? I thought the whole >> point of clang-cl was to be

[PATCH] D82862: [ThinLTO] Always parse module level inline asm with At dialect

2021-01-21 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D82862#2500717 , @hans wrote: >> In D82862#2498785 , @hans wrote: >> >>> The motivation for my change was really just to make ThinLTO compiles work >>> the same as non-ThinLTO ones.

[PATCH] D82862: [ThinLTO] Always parse module level inline asm with At dialect

2021-01-15 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D82862#2500038 , @pengfei wrote: >> What is the reason for treating this differently in LLVM? > > I'm not sure if it is related to this. I think one difference is that LLVM is > supporting MS inline assembly. Although it uses

[PATCH] D82862: [ThinLTO] Always parse module level inline asm with At dialect

2021-01-14 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Herald added a subscriber: pengfei. Hi @hans , we're having some issues with using the AssemblerDialect mechanism to support both the GNU assembler and the IBM HLASM assembler in the SystemZ back-end. See also the discussion started here:

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

2020-10-16 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D87279#2334510 , @jonpa wrote: > The problem seems to be with a tied earlyclobber operand: > > asm("" : "+"(a)); > > Per the comment in InlineAsm::ConstraintInfo::Parse(), only output can be > earlyclobber. > > I am not

[PATCH] D84341: Implement __builtin_eh_return_data_regno for SystemZ

2020-07-24 Thread Ulrich Weigand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7f003957bfcd: [SystemZ] Implement __builtin_eh_return_data_regno (authored by uweigand). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84341/new/

[PATCH] D84341: Implement __builtin_eh_return_data_regno for SystemZ

2020-07-22 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM with the format fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84341/new/ https://reviews.llvm.org/D84341

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-07-12 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D80833#2123508 , @aganea wrote: > In D80833#2109172 , @uweigand wrote: > > > Hmm, with clang-cl it seems the driver is trying to use this: > > Target: s390x-pc-windows-msvc > > which

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-10 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand closed this revision. uweigand added a comment. Committed as 4c5a93bd58bad70e91ac525b0e020bd5119a321a . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81583/new/ https://reviews.llvm.org/D81583

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-09 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand updated this revision to Diff 276650. uweigand added a comment. Drop AllowNoUniqueAddress parameter; address review comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81583/new/ https://reviews.llvm.org/D81583 Files: clang/lib/CodeGen/TargetInfo.cpp

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-08 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D81583#2138127 , @qiucf wrote: > Thanks for this patch! > > If I understand correctly, only `isEmptyRecord`/`isEmptyField` and places > checking any field is zero bit-width may need change for this? Since >

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-08 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand marked 2 inline comments as done. uweigand added a comment. In D81583#2137277 , @efriedma wrote: > I'm tempted to say this is a bugfix for the implementation of > no_unique_address, and just fix it globally for all ABIs. We're never going > to

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-08 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand marked 6 inline comments as done. uweigand added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:521 + // [[no_unique_address]] attribute (since C++20). Those do count + // as empty according to the Itanium ABI. This property is currently + // only

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-08 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand updated this revision to Diff 276414. uweigand added a comment. Handle array of empty records correctly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81583/new/ https://reviews.llvm.org/D81583 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGen/systemz-abi.cpp

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-07 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand updated this revision to Diff 276151. uweigand added a comment. Rebased against mainline. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81583/new/ https://reviews.llvm.org/D81583 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGen/systemz-abi.cpp Index:

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-07-07 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Another ping ... See also http://lists.llvm.org/pipermail/cfe-dev/2020-July/066162.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81583/new/ https://reviews.llvm.org/D81583

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-23 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Hmm, with clang-cl it seems the driver is trying to use this: Target: s390x-pc-windows-msvc which of course doesn't exist. Not sure what is supposed to be happening here, but it seems that it's falling back on s390x-linux since on s390x, Linux is currently the only

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-06-23 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Ping? I'd really appreciate feedback about this ABI issue, in particular from other affected target maintainers ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81583/new/ https://reviews.llvm.org/D81583

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-23 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. > Line 4 here fails on s390x but not on other Unix flavors, see: > http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33346/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adebug-info-codeview-buildinfo.c > > @thakis @uweigand Any ideas would could go wrong

[PATCH] D78644: [LSan] Enable for SystemZ

2020-06-15 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. This all looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78644/new/ https://reviews.llvm.org/D78644

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

2020-06-10 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand created this revision. uweigand added reviewers: craig.topper, erichkeane, jasonliu, kbarton, rnk, asl, sunfish, t.p.northover, arsenm, asb. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. The SystemZ ABI has special cases to handle structs containing just a

[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

2020-03-31 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Thanks for working on this, @thakis ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list

[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

2020-03-30 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Ah, good point. Dimitry, can you prepare an updated patch to implement Jonas' suggestion? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914

[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

2020-03-30 Thread Ulrich Weigand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9c9d88d8b1bb: [SystemZ] Allow configuring default CLANG_SYSTEMZ_ARCH (authored by uweigand). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

2020-02-10 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D72675#1829866 , @hfinkel wrote: > > I'm not sure whether this is deliberate (but it seems weird) or just a bug. > > I can ask the GCC developers ... > > Please do. If there's a rationale, we should know. Sorry for the

[PATCH] D74146: [SytemZ] Disable vector ABI when using option -march=arch[8|9|10]

2020-02-07 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74146/new/ https://reviews.llvm.org/D74146 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72906: [X86] Improve X86 cmpps/cmppd/cmpss/cmpsd intrinsics with strictfp

2020-01-24 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D72906#1837905 , @craig.topper wrote: > In D72906#1826849 , @uweigand wrote: > > > In D72906#1826122 , @craig.topper > > wrote: > > > > > In

[PATCH] D72722: [FPEnv] [SystemZ] Platform-specific builtin constrained FP enablement

2020-01-21 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72722/new/ https://reviews.llvm.org/D72722 ___ cfe-commits mailing list

[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

2020-01-20 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. I've had a quick look at GCC, and it seems there's a couple of different issues. First of all, `-ffast-math` is a "collective" flag that has no separate meaning except setting a bunch of other flags. Currently, these flags are: `-fno-math-errno`, `

[PATCH] D72906: [X86] Improve X86 cmpps/cmppd/cmpss/cmpsd intrinsics with strictfp

2020-01-17 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D72906#1826122 , @craig.topper wrote: > In D72906#1826061 , @uweigand wrote: > > > > The constrained fcmp intrinsics don't allow the TRUE/FALSE predicates. > > > > Hmm, maybe they

[PATCH] D72906: [X86] Improve X86 cmpps/cmppd/cmpss/cmpsd intrinsics with strictfp

2020-01-17 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. > The constrained fcmp intrinsics don't allow the TRUE/FALSE predicates. Hmm, maybe they should then? The only reason I didn't add them initially was that I wasn't sure they were useful for anything; if they are, it should be straightforward to add them back.

[PATCH] D72722: [FPEnv] [SystemZ] Platform-specific builtin constrained FP enablement

2020-01-16 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. > What are the semantics of vfnmadb with respect to when it rounds vs the > negation? Hmm, interesting. The z/Architecture Principles of Operation states: > The results for each element of VECTOR FP NEGA- > TIVE MULTIPLY AND ADD and VECTOR FP NEGA- > TIVE MULTIPLY

[PATCH] D72722: [FPEnv] [SystemZ] Platform-specific builtin constrained FP enablement

2020-01-16 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. A few comments (see inline) -- otherwise this looks good to me, thanks! Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13354 + return Builder.CreateFNeg(Builder.CreateConstrainedFPCall(F, {X, Y, Z}), "neg"); + +} else {

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-15 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D71467#1820589 , @rjmccall wrote: > I think I have a slight preference for the second option, where there's a > single method that does all the work for the two cases. OK, now checked in as 870137d

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-14 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D71467#1817939 , @rjmccall wrote: > Is this approach going to work with scope-local strictness? We need a way to > do a comparison that has the non-strict properties but appears in a function > that enables strictness

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-10 Thread Ulrich Weigand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG76e9c2a9870e: [FPEnv] Generate constrained FP comparisons from clang (authored by uweigand). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71467/new/

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-08 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Ping again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71467/new/ https://reviews.llvm.org/D71467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D71854: [SystemZ] Use FNeg in s390x clang builtins

2020-01-02 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71854/new/ https://reviews.llvm.org/D71854 ___ cfe-commits mailing list

[PATCH] D71854: [SystemZ] Use FNeg in s390x clang builtins

2019-12-24 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Otherwise this LGTM. Thanks for taking care of those! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71854/new/ https://reviews.llvm.org/D71854 ___ cfe-commits mailing list

[PATCH] D71854: [SystemZ] Use FNeg in s390x clang builtins

2019-12-24 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand requested changes to this revision. uweigand added a comment. This revision now requires changes to proceed. This also needs updating the test cases that are testing for the old behavior: Failing Tests (4): Clang :: CodeGen/builtins-systemz-vector.c Clang ::

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-18 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Ping? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71467/new/ https://reviews.llvm.org/D71467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71669: [Clang FE, SystemZ] Don't add "true" value for the "mnop-mcount" attribute.

2019-12-18 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71669/new/ https://reviews.llvm.org/D71669 ___ cfe-commits mailing list

[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

2019-12-17 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. It looks like this caused build bot failures: http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/28928/steps/ninja%20check%201/logs/FAIL%3A%20lit%3A%3A%20shtest-format.py

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D71467#1786192 , @erichkeane wrote: > __builtin_fpclassify/isfinite/isinf/isinf_sign/isnan/isnormal/signbit are all > implemented the same as the OTHER ones, except there is a strange fixup step > in SEMA that removes the

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D71467#1785943 , @erichkeane wrote: > I did the compare operators that didn't work right, and will do a separate > patch for the fp-classification type ones: > f02d6dd6c7afc08f871a623c0411f2d77ed6acf8 >

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand updated this revision to Diff 234096. uweigand added a comment. Added float (f32) test cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71467/new/ https://reviews.llvm.org/D71467 Files: clang/lib/CodeGen/CGExprScalar.cpp clang/test/CodeGen/fpconstrained-cmp-double.c

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-13 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand created this revision. uweigand added reviewers: kpn, andrew.w.kaylor, craig.topper, cameron.mcinally, RKSimon, spatel, rjmccall, rsmith. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Update the IRBuilder to generate constrained FP comparisons

[PATCH] D71049: Remove implicit conversion that promotes half to other larger precision types for fp classification builtins

2019-12-10 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. This causes build bot failures on SystemZ due to a failed assertion in ConstantFP::getInfinity: http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/28723/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Abuiltins.c Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D67763: [Clang FE] Recognize -mnop-mcount CL option

2019-11-04 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand accepted this revision. uweigand added a comment. This revision is now accepted and ready to land. This looks good to me now. Given that this patch implements an option only relevant on SystemZ, this should be OK. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67763/new/

[PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

2019-09-26 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. This makes sense to me (although we don't currently need the options parameter there). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66795/new/ https://reviews.llvm.org/D66795 ___ cfe-commits mailing list

[PATCH] D63366: AMDGPU: Add GWS instruction builtins

2019-06-19 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. I'm now getting test suite failures like this: /home/uweigand/llvm/llvm-head/tools/clang/test/CodeGenOpenCL/builtins-amdgcn.cl:554:3: error: cannot compile this builtin function yet __builtin_amdgcn_ds_gws_init(value, id); Is this supposed to work? I'm not

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-05-29 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. As of r361920, the SystemZ build bots are green again. Thanks, Eric! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-05-27 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. Looks like this test is failing on SystemZ since it was added, making all our build bots red: /home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/CodeGenCXX/builtin_FUNCTION.cpp:9:11: error: CHECK: expected string not found in input // CHECK:

[PATCH] D55916: [clang] Replace getOS() == llvm::Triple::*BSD with isOS*BSD() [NFCI]

2018-12-20 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D55916#1337427 , @uweigand wrote: > This causes test case failures due to no longer linking with -lrt on Linux. Never mind, already fixed :-) Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D55916: [clang] Replace getOS() == llvm::Triple::*BSD with isOS*BSD() [NFCI]

2018-12-20 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. This causes test case failures due to no longer linking with -lrt on Linux. Comment at: lib/Driver/ToolChains/CommonArgs.cpp:606 CmdArgs.push_back("-lpthread"); -if (TC.getTriple().getOS() != llvm::Triple::OpenBSD) +if

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-12-04 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. As an aside, it would be nice if we had a test case that verifies the explicit values of alignof(max_align_t) on all supported platforms. This is an ABI property that should never change. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-12-04 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand marked an inline comment as done. uweigand added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 uweigand wrote: >

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-12-04 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 jyknight wrote: > EricWF wrote: > > uweigand wrote: > > >

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-12-03 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 jyknight wrote: > uweigand wrote: > > jyknight wrote: > > >

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2018-12-03 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In D53157#1309743 , @cameron.mcinally wrote: > Digressing a bit, but has anyone given thought to how this implementation > will play out with libraries? When running with traps enabled, libraries must > be compiled for

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-11-29 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 jyknight wrote: > Can you fix clang to consistently define

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2018-11-21 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment. In https://reviews.llvm.org/D53157#1305233, @kpn wrote: > In https://reviews.llvm.org/D53157#1304347, @uweigand wrote: > > > But given that there is still infrastructure missing in the IR optimizers, > > I also think that at least in the first implementation, we

  1   2   >