Page MenuHomePhabricator

uweigand (Ulrich Weigand)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 14 2013, 11:48 AM (382 w, 3 d)

Recent Activity

Today

uweigand accepted D85822: [Sanitizer] Fix segfaults during unwinding on SystemZ.

Yes, this makes sense to me as well. Patch LGTM.

Wed, Aug 12, 11:43 AM · Restricted Project

Mon, Aug 10

uweigand added a comment to D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui.

This doesn't look correct. As far as I can see, none of the conversion functions were actually changed to handle strict operations. For one, you'll need strict variants of all the PowerPC-specific conversion operations, use them in all the conversion subroutines, and consistently track their chain nodes.

The patch only adds a strict variant of the direct move, which seems to me the only operation where actually a strict version is not required ...

Thanks for pointing them out! I have something unclear about chains:

(1) If a constrained operation is expanded into several FP nodes a-b-c, they should all have chain set to former operation (b's chain is a, c's chain is b) even if they have def relationship?

Mon, Aug 10, 10:27 AM · Restricted Project

Tue, Aug 4

uweigand added a comment to D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui.

This doesn't look correct. As far as I can see, none of the conversion functions were actually changed to handle strict operations. For one, you'll need strict variants of all the PowerPC-specific conversion operations, use them in all the conversion subroutines, and consistently track their chain nodes.

Tue, Aug 4, 5:50 AM · Restricted Project

Mon, Aug 3

uweigand accepted D81906: [CodeGen] Expand float operand for STRICT_FSETCC/STRICT_FSETCCS.

This LGTM now.

Mon, Aug 3, 3:49 AM · Restricted Project
uweigand added a comment to D84835: [APFloat] Fix incorrect fptrunc rouning when from semantics exp lower than minimal exp of target semantics.

I'm not sure what specific problem you're trying to solve here, but this change in itself looks incorrect.

Mon, Aug 3, 3:40 AM · Restricted Project

Thu, Jul 30

uweigand added a comment to D84764: Fix computeHostNumPhysicalCores() for Linux on POWER and Linux on Z.

Sure, of course assuming the test still passes.

Thu, Jul 30, 2:40 AM · Restricted Project

Wed, Jul 29

uweigand accepted D84764: Fix computeHostNumPhysicalCores() for Linux on POWER and Linux on Z.

LGTM for SystemZ as well.

Wed, Jul 29, 10:38 AM · Restricted Project

Fri, Jul 24

uweigand committed rG7f003957bfcd: [SystemZ] Implement __builtin_eh_return_data_regno (authored by uweigand).
[SystemZ] Implement __builtin_eh_return_data_regno
Fri, Jul 24, 1:29 AM
uweigand closed D84341: Implement __builtin_eh_return_data_regno for SystemZ.
Fri, Jul 24, 1:29 AM · Restricted Project

Thu, Jul 23

uweigand committed rG68a80a4436c6: [SystemZ] Ensure -mno-vx disables any use of vector features (authored by uweigand).
[SystemZ] Ensure -mno-vx disables any use of vector features
Thu, Jul 23, 6:35 AM

Wed, Jul 22

uweigand accepted D84341: Implement __builtin_eh_return_data_regno for SystemZ.

LGTM with the format fixes.

Wed, Jul 22, 10:29 AM · Restricted Project

Tue, Jul 21

uweigand accepted D83654: [PowerPC] Support constrained vector fp/int conversion.

This LGTM now.

Tue, Jul 21, 12:46 AM · Restricted Project

Fri, Jul 17

uweigand added inline comments to D83654: [PowerPC] Support constrained vector fp/int conversion.
Fri, Jul 17, 3:47 AM · Restricted Project
uweigand requested changes to D81918: [PowerPC] Support lowering int-to-fp on ppc_fp128.

See inline comments about chain handling.

Fri, Jul 17, 3:43 AM · Restricted Project

Thu, Jul 16

uweigand accepted D81727: [PowerPC] Support constrained fp operation for setcc.

The one issue I still see is that when falling back to library calls on older platforms, signaling and non-signaling compares both are treated the same. But that's really a pre-existing problem because this is not fully supported by libgcc (see the comment in TargetLowering::softenSetCCOperands), so it's not really a problem with this patch.

Thu, Jul 16, 5:19 AM · Restricted Project

Tue, Jul 14

uweigand added inline comments to D83654: [PowerPC] Support constrained vector fp/int conversion.
Tue, Jul 14, 4:00 AM · Restricted Project

Jul 12 2020

uweigand added a comment to D80833: [CodeView] Add full repro to LF_BUILDINFO record.

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 supported OS.

I'm seeing some of the tests are setting the target explicitly %clang_cl --target=x86_64-windows-msvc. Would that work on your machine? Or should I do UNSUPPORTED: s390x ?

Jul 12 2020, 3:29 PM · Restricted Project, Restricted Project

Jul 10 2020

uweigand closed D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

Committed as 4c5a93bd58bad70e91ac525b0e020bd5119a321a.

Jul 10 2020, 5:12 AM · Restricted Project
uweigand committed rG4c5a93bd58ba: [ABI] Handle C++20 [[no_unique_address]] attribute (authored by uweigand).
[ABI] Handle C++20 [[no_unique_address]] attribute
Jul 10 2020, 5:01 AM

Jul 9 2020

uweigand updated the diff for D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

Drop AllowNoUniqueAddress parameter; address review comment.

Jul 9 2020, 12:50 AM · Restricted Project

Jul 8 2020

uweigand committed rGcca8578efab0: [SystemZ] Allow specifying integer registers as part of the address calculation (authored by uweigand).
[SystemZ] Allow specifying integer registers as part of the address calculation
Jul 8 2020, 9:21 AM
uweigand closed D83251: [SystemZ] Allow specifying integer registers as part of the address calculation.
Jul 8 2020, 9:20 AM · Restricted Project
uweigand added a comment to D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

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 isSingleElementStruct and isHomogeneousAggregate just use them to skip empty fields in aggregate. I didn't see direct checking for empty fields on PowerPC. So all change needed on PPC seems to be generic. By enabling AllowNoUniqueAddr in these methods, case in https://lists.llvm.org/pipermail/llvm-dev/2020-July/143141.html can be correctly recognized as homogeneous aggregate.

Jul 8 2020, 7:10 AM · Restricted Project
uweigand added a comment to D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

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 get anything done here if we require a separate patch for each ABI variant clang supports.

Jul 8 2020, 7:08 AM · Restricted Project
uweigand added inline comments to D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.
Jul 8 2020, 7:07 AM · Restricted Project
uweigand updated the diff for D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

Handle array of empty records correctly.

Jul 8 2020, 6:59 AM · Restricted Project
uweigand accepted D83376: [Legalizer] Fix wrong operand in split vector helper.

Yes, this does look like an obvious typo. LGTM!

Jul 8 2020, 5:00 AM · Restricted Project

Jul 7 2020

uweigand updated the diff for D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

Rebased against mainline.

Jul 7 2020, 11:26 AM · Restricted Project
uweigand committed rG80a1b95b8e72: [SystemZ ABI] Allow class types in GetSingleElementType (authored by uweigand).
[SystemZ ABI] Allow class types in GetSingleElementType
Jul 7 2020, 10:57 AM
uweigand abandoned D74601: SystemZ target - Incorrect code generated for accessing thread_local variables when high-word feature is enabled.
Jul 7 2020, 5:27 AM · Restricted Project
uweigand commandeered D74601: SystemZ target - Incorrect code generated for accessing thread_local variables when high-word feature is enabled.
Jul 7 2020, 5:27 AM · Restricted Project
uweigand added a comment to D74601: SystemZ target - Incorrect code generated for accessing thread_local variables when high-word feature is enabled.

This was actually fixed in a different way here: https://reviews.llvm.org/D75014
I'm closing this review now.

Jul 7 2020, 5:27 AM · Restricted Project
uweigand added a comment to D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

Another ping ...

Jul 7 2020, 5:25 AM · Restricted Project
uweigand accepted D83251: [SystemZ] Allow specifying integer registers as part of the address calculation.

A couple of cosmetic changes inline, otherwise the LGTM. Thanks!

Jul 7 2020, 2:16 AM · Restricted Project

Jun 30 2020

uweigand added a comment to D82622: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'..

This commit causes test case failures on s390x (unfortunately hidden as the build bot was already red due to an unrelated issue):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624

http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3ADWARF-debug_info.yaml

At first glance, seems like this could be endian-related?

The failed tests are removed in 8032727a43ca678b0b923abaa04638f500a060d6. Thanks.

Jun 30 2020, 9:11 AM · Restricted Project, Restricted Project
uweigand added a comment to D82602: [SelectionDAG] don't split branch on logic-of-vector-compares.

I've now updated the knownbits.ll test case to make it less fragile and still test what it is supposed to test (commit e9c6b63). The problem was use of "undef" (which makes it susceptible to collapse as generic optimizations improve), and the fact that knownBits was operating at the very edge of MaxRecursionDepth, so changes in common code could push it above the limit.

Jun 30 2020, 8:06 AM · Restricted Project
uweigand committed rGe9c6b63d4a16: [SystemZ] Simplify knownbits.ll test (authored by uweigand).
[SystemZ] Simplify knownbits.ll test
Jun 30 2020, 7:35 AM
uweigand added a comment to D82622: [DWARFYAML][debug_info] Replace 'InitialLength' with 'Format' and 'Length'..

This commit causes test case failures on s390x (unfortunately hidden as the build bot was already red due to an unrelated issue):
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33624

Jun 30 2020, 5:55 AM · Restricted Project, Restricted Project
uweigand requested changes to D81727: [PowerPC] Support constrained fp operation for setcc.
Jun 30 2020, 3:14 AM · Restricted Project
uweigand requested changes to D81906: [CodeGen] Expand float operand for STRICT_FSETCC/STRICT_FSETCCS.
Jun 30 2020, 3:14 AM · Restricted Project
uweigand added a comment to D81918: [PowerPC] Support lowering int-to-fp on ppc_fp128.

For unsigned conversion, the input is treated as signed here, and the result fixed up by adding 2^N; something along the lines of

(signed)N >= 0 ? signed_to_FP((signed)N) : signed_to_FP(N - 2^N) + (FP)2^N
Jun 30 2020, 2:08 AM · Restricted Project
uweigand accepted D82794: [SystemZ] Add NoMerge MIFlag.

LGTM, thanks!

Jun 30 2020, 12:30 AM · Restricted Project

Jun 29 2020

uweigand updated subscribers of D82322: [ASan][MSan] Remove EmptyAsm and set the CallInst to nomerge to avoid from merging..

It looks like this broke the s390x build bots, since then I'm seeing a lot of ASan and MSan errors:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33485

It seems like all those tests failed because the second CHECK should match output line 4 but missing the line number at the end.
Since the patch passed tests in all other build bots, I wonder if it's because the build bot config is flaky.

Jun 29 2020, 10:48 AM · Restricted Project
uweigand added a comment to D82322: [ASan][MSan] Remove EmptyAsm and set the CallInst to nomerge to avoid from merging..

It looks like this broke the s390x build bots, since then I'm seeing a lot of ASan and MSan errors:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33485

Jun 29 2020, 6:26 AM · Restricted Project

Jun 26 2020

uweigand added a comment to D82602: [SelectionDAG] don't split branch on logic-of-vector-compares.
  1. I missed a SystemZ test change in the earlier draft; adding SystemZ reviewers to confirm if that's an improvement.
Jun 26 2020, 8:44 AM · Restricted Project
uweigand added inline comments to D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui.
Jun 26 2020, 5:56 AM · Restricted Project

Jun 25 2020

uweigand added inline comments to D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui.
Jun 25 2020, 9:07 AM · Restricted Project

Jun 24 2020

uweigand added inline comments to D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui.
Jun 24 2020, 7:31 AM · Restricted Project

Jun 23 2020

uweigand added a comment to D80833: [CodeView] Add full repro to LF_BUILDINFO record.

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 supported OS.

Jun 23 2020, 7:58 AM · Restricted Project, Restricted Project
uweigand added a comment to D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.

Ping? I'd really appreciate feedback about this ABI issue, in particular from other affected target maintainers ...

Jun 23 2020, 1:32 AM · Restricted Project
uweigand added a comment to D80833: [CodeView] Add full repro to LF_BUILDINFO record.

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 here?

Jun 23 2020, 1:32 AM · Restricted Project, Restricted Project

Jun 16 2020

uweigand accepted D81671: [SystemZ] Bugfix in storeLoadCanUseBlockBinary().

Shouldn't the check be done inside canUseBlockOperation? I believe the other user (storeLoadCanUseMVC) really needs the same check.

My understanding is that storeLoadCanUseBlockBinary() is used as a pattern matching predicate for block memory operations of AND/OR/XOR. These involve two loads and one store. While canUseBlockOperation() has checks for a pair of a store and the load (of the other address), the second load (to the same address as the store) also needs to be checked, which is what this patch does.

The other user of canUseBlockOperation() is storeLoadCanUseMVC() which however only involves one load and one store, and this is why there is no need for anything else in this case.

Jun 16 2020, 1:04 AM · Restricted Project

Jun 15 2020

uweigand accepted D78644: [LSan] Enable for SystemZ.

This all looks good to me.

Jun 15 2020, 9:45 AM · Restricted Project, Restricted Project
uweigand added a comment to D81671: [SystemZ] Bugfix in storeLoadCanUseBlockBinary().

Shouldn't the check be done inside canUseBlockOperation? I believe the other user (storeLoadCanUseMVC) really needs the same check.

Jun 15 2020, 6:28 AM · Restricted Project

Jun 10 2020

uweigand created D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute.
Jun 10 2020, 9:17 AM · Restricted Project

Jun 5 2020

uweigand added a comment to D78488: [SystemZ] Emit VLLEZ from tryShuffleWithZeroVec().

OK. I agree this can be abandoned now. Thanks!

Jun 5 2020, 8:52 AM · Restricted Project
uweigand accepted D78486: [SystemZ] Expand vector zero-extend into a shuffle..

This looks all good to me now, we should verify that performance results are still good, and then it can go in.

Jun 5 2020, 8:52 AM · Restricted Project
uweigand accepted D78717: [SystemZ] Implement -fstack-clash-protection.

A few cosmetic issues mentioned inline, otherwise this now looks good to me. Thanks!

Jun 5 2020, 8:51 AM · Restricted Project, Restricted Project

Jun 4 2020

uweigand accepted D80329: [SVE] Eliminate calls to default-false VectorType::get() from SystemZ.

LGTM.

Jun 4 2020, 2:40 AM · Restricted Project

May 18 2020

uweigand accepted D79925: [SystemZ] Eliminate the need to create a zero vector by reusing the mask..

LGTM, thanks!

May 18 2020, 12:26 PM · Restricted Project
uweigand added a comment to D79925: [SystemZ] Eliminate the need to create a zero vector by reusing the mask..

It seems that now MachineCSE can remove a few VPERMs also. At least one case was because instead of using an undef source operand, the mask was used (it doesn't help any to replace an undef Ops[1] with Op2 on the last line of getGeneralPermuteNode(), which I first thought). The test case I have reduced (not included) shows that MCSE on trunk fails to remove the second vperm if even though the instructions are near identical. The only difference to the success with this patch is that instead of the reused mask, there is an IMPLICIT_DEF. It looks to me that this is a minor deficiency of MCSE (Two different vregs defined by IMPLICIT_DEF should not stop CSE, I would think).

isZeroOrUndefVector(): The check for the undef vector used to be beneficial for the zero_extend_vector_inreg patch, but the way it looks now (just using a single unpack), it is not needed for anymore (NFC). I think it could be removed or we can keep it here to get the few less vperms...

Two tests - one for each case handled. Not quite sure what happened with vector-constrained-fp-intrinsics.ll - I don't understand why v0 is no longer used, but it seems that this patch begins to change things when the second operand of the VPERM is undefined.

May 18 2020, 8:34 AM · Restricted Project
uweigand added a comment to D78717: [SystemZ] Implement -fstack-clash-protection.

Normally, I would have expected a pseudo MachineInstruction to be built here with an immediate operand for the size requirement...

That sounds very nice. If you go that way I'll happily update the X86 code accordingly. Or If you point me to some example / doc / code on pseudo MachineInstruction, I can implement that too.

@uweigand : Would you rather use a target pseudo instruction for this rather than using a call with metadata?

May 18 2020, 5:52 AM · Restricted Project, Restricted Project

May 12 2020

uweigand added a comment to D49132: Fix gcov profiling on big-endian machines.

Hmm, I still think the read/write_64bit_value change is correct, GCC does read/write 64-bit counter values as lo/hi pairs even on big-endian systems.

May 12 2020, 1:02 AM

May 11 2020

uweigand accepted D76705: [SystemZ] Improve foldMemoryOperandImpl: vec->FP conversions.

Can we instead move the W->FP mnemonic conversion back into BinaryVRRa/TernaryVRRe instead, so that the OpKey for all these vector instructions consistently holds the FP-reg instruction name? The MemFold-pseudos would then have the same mnemonic as OpKey and MemKey.
... Ah, that still runs into the same name collisions with the MemFold pattern as you pointed out earlier. Sorry for missing that!

Yeah, WFADB:ADBR would map to ADB, and not ADB_MemFoldPseudo which is what we need...

So if we use an extra modifier in the OpKey for those case, then we're basically back to your version of Apr 28, 7:15 PM, except possibly with the explicit mnemonics in the .td file.

Ok, went back to that version plus added the explicit fp_mnemonic fields, which seems nice to me as they eliminate those ugly string substitutions.

May 11 2020, 8:34 AM · Restricted Project

May 8 2020

uweigand added a comment to D78486: [SystemZ] Expand vector zero-extend into a shuffle..

So now I'm wondering instead: why does your new code ever trigger in the first place; why isn't this already handled in GeneralShuffle::add?

I think that be two different cases, where my handling is addressing a *new* permute resulting from combining two operands, when trying to combine (unpack) a third source operand with that first (new) permute.

May 8 2020, 12:52 PM · Restricted Project
uweigand added a comment to D76705: [SystemZ] Improve foldMemoryOperandImpl: vec->FP conversions.

Hmm, talking about MemFold, I'm wondering about this:
​ // Fused multiply and add/sub need to have the same dst and accumulator reg.

May 8 2020, 12:52 PM · Restricted Project
uweigand added a comment to D76705: [SystemZ] Improve foldMemoryOperandImpl: vec->FP conversions.

Ah, that still runs into the same name collisions with the MemFold pattern as you pointed out earlier. Sorry for missing that!

May 8 2020, 12:20 PM · Restricted Project
uweigand added a comment to D76705: [SystemZ] Improve foldMemoryOperandImpl: vec->FP conversions.

Reverted back to the first version of new instruction classes providing the mapping from W...-reg -> FP-mem instructions.

May 8 2020, 9:37 AM · Restricted Project
uweigand added a comment to D78717: [SystemZ] Implement -fstack-clash-protection.

Added probing of the tail of dyn-alloca, with an extra check for zero tail, roughly like GCC. It seems it can be assumed that the alloca size will always be a multiple of 8 bytes, or? I think that is necessary, or there might be final probe partially below SP (if tail is e.g. 4 bytes). Also, if the stack-probe size is 0, an infinite loop would result (assert?), but I suppose that would always be noticeable.

May 8 2020, 9:37 AM · Restricted Project, Restricted Project

May 6 2020

uweigand committed rG947f78ac27f4: [SystemZ] Fix/optimize vec_load_len and related intrinsics (authored by uweigand).
[SystemZ] Fix/optimize vec_load_len and related intrinsics
May 6 2020, 12:27 PM

May 4 2020

uweigand added a comment to D78486: [SystemZ] Expand vector zero-extend into a shuffle..

I'm wondering: unless I'm missing something, there's still one specific case where you generate a vperm followed by an unpack (the case where you already had a permute as source). Wouldn't it be preferable to just use a single vperm there as well?

This is the case where the input is a permute that has no other users, which means that it's being replaced. So instead of vperm + vperm, we get a vperm + unpack, which follows the same reasoning of replacing a vperm with a single unpack.

For example, in the case with three source ops, where A and B first are combined with a permute, and now AB and C are to be combined: instead of permuting AB and C, the AB permute is changed so that AB and C can be unpacked.

May 4 2020, 12:21 PM · Restricted Project
uweigand added a comment to D78717: [SystemZ] Implement -fstack-clash-protection.

gcc seems not to be probing the residual allocation after the loop. However if only two (unrolled) allocations were made, the residual is also probed.

I'm not seeing this, do you have an example?

void large_stack() {
  volatile int stack[2000], i;
  for (i = 0; i < sizeof(stack) / sizeof(int); ++i)
    stack[i] = i;
}

With stack[2000], I see

aghi    %r15,-4096
cg      %r0,4088(%r15)
aghi    %r15,-4072
cg      %r0,4064(%r15)

With stack[8000], I don't see a probe after the loop...

May 4 2020, 10:09 AM · Restricted Project, Restricted Project
uweigand added a comment to D79283: [PowerPC] Add missing handling for half precision.

This is missing from the SystemZ back end

May 4 2020, 6:22 AM · Restricted Project

Apr 30 2020

uweigand added a comment to D78486: [SystemZ] Expand vector zero-extend into a shuffle..

It certainly seems to be an improvement on two benchmarks to do just either one unpack or else a vperm (and not multiple unpacks). In fact, i505.mcf actually regressed (2.5%) when doing two unpacks instead of a vperm (with -ffp-contract=fast). So the initial idea of reducing the number of vperms seems to have been proven wrong - it is better to have a single vperm on the critical path rather than multiple unpacks.

Apr 30 2020, 11:47 AM · Restricted Project

Apr 29 2020

uweigand committed rGe1de2773a534: [SystemZ] Allow specifying plain register numbers in AsmParser (authored by uweigand).
[SystemZ] Allow specifying plain register numbers in AsmParser
Apr 29 2020, 11:50 AM
uweigand committed rG6bfde063f0a7: [SystemZ] Simplify register parsing in AsmParser (authored by uweigand).
[SystemZ] Simplify register parsing in AsmParser
Apr 29 2020, 11:50 AM
uweigand added a comment to D76705: [SystemZ] Improve foldMemoryOperandImpl: vec->FP conversions.

> Is this better than previously?

Apr 29 2020, 3:43 AM · Restricted Project

Apr 28 2020

uweigand committed rGc90e09b13c90: [SystemZ] Use reserved keywords in vecintrin.h (authored by uweigand).
[SystemZ] Use reserved keywords in vecintrin.h
Apr 28 2020, 10:12 AM
uweigand committed rG095ccf445565: [SystemZ] Avoid __INTPTR_TYPE__ conversions in vecintrin.h (authored by uweigand).
[SystemZ] Avoid __INTPTR_TYPE__ conversions in vecintrin.h
Apr 28 2020, 10:12 AM
Herald added a project to D78717: [SystemZ] Implement -fstack-clash-protection: Restricted Project.

Not looking at the code so far, but just answering your questions:

Apr 28 2020, 9:06 AM · Restricted Project, Restricted Project
uweigand added a comment to D76705: [SystemZ] Improve foldMemoryOperandImpl: vec->FP conversions.

Looks good to me now, just one cosmetic comment inline.

Apr 28 2020, 8:34 AM · Restricted Project
uweigand added a comment to D76705: [SystemZ] Improve foldMemoryOperandImpl: vec->FP conversions.

Maybe there should be a test that checks that a live CC value isn't clobbered by introducing an FP-mem instruction, such as ADB...

Apr 28 2020, 8:33 AM · Restricted Project

Apr 24 2020

uweigand added a comment to D78486: [SystemZ] Expand vector zero-extend into a shuffle..

Hi Jonas, I haven't looked into everything in detail, but first one fundmental question: My understanding was that the current GeneralShuffle code would detect the shuffle you generate for a zero-extend as a case of a MERGE. Later, combineMERGE would detect that one input of the MERGE is a zero vector, and replace the merge by an UNPACKL.

Apr 24 2020, 8:04 AM · Restricted Project

Apr 15 2020

uweigand accepted D78187: [SystemZ] Bugfix in adjustSubwordCmp().

LGTM, thanks!

Apr 15 2020, 3:47 AM · Restricted Project

Apr 9 2020

uweigand added a comment to D76624: [MSan] Add instrumentation for SystemZ.

Thanks!

Apr 9 2020, 4:50 AM · Restricted Project
uweigand added a comment to D76624: [MSan] Add instrumentation for SystemZ.

Just a minor cosmetic suggestion inline. Otherwise the Z ABI parts now all LGTM.

Apr 9 2020, 3:45 AM · Restricted Project

Apr 3 2020

uweigand added a comment to D76624: [MSan] Add instrumentation for SystemZ.

The Z ABI implementation now looks correct to me, except for one corner case: an LLVM-level argument type of f128 is passed via implicit reference. Now, in most cases this is already handled at the clang level, i.e. when you have a "long double" at C source level, you'll already see a pointer type in the LLVM IR instead. However, there are still a few cases where there is a f128 at the LLVM IR level, e.g. as arguments to some compiler builtin / libgcc routines. Those are only transformed to implicit reference in the LLVM back-end, so I believe for completeness this case should also be handled here.

Apr 3 2020, 11:20 AM · Restricted Project

Mar 31 2020

uweigand committed rGc726c920e040: [SystemZ] Allow %r0 in address context for AsmParser (authored by uweigand).
[SystemZ] Allow %r0 in address context for AsmParser
Mar 31 2020, 10:53 AM
uweigand added a comment to D71938: [SCCP] Use constant ranges for casts..

I'm now getting an assertion failure in the LNT test suite:
http://lab.llvm.org:8011/builders/clang-s390x-linux-lnt/builds/17826/steps/test-suite/logs/stdio

Mar 31 2020, 8:16 AM · Restricted Project
uweigand accepted D76771: [SystemZ] Improve foldMemoryOperandImpl: MS(G)RKC -> MS(G)C.

LGTM, thanks!

Mar 31 2020, 7:09 AM · Restricted Project
uweigand added inline comments to D76771: [SystemZ] Improve foldMemoryOperandImpl: MS(G)RKC -> MS(G)C.
Mar 31 2020, 6:04 AM · Restricted Project
uweigand added a comment to D76771: [SystemZ] Improve foldMemoryOperandImpl: MS(G)RKC -> MS(G)C.

Thanks for running the benchmark, I guess I'm OK with the current implementation then.

Mar 31 2020, 12:31 AM · Restricted Project
uweigand added a comment to D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH.

Thanks for working on this, @thakis !

Mar 31 2020, 12:30 AM · Restricted Project, Restricted Project

Mar 30 2020

uweigand added a comment to D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH.

Ah, good point. Dimitry, can you prepare an updated patch to implement Jonas' suggestion?

Mar 30 2020, 6:27 AM · Restricted Project, Restricted Project
uweigand committed rG9c9d88d8b1bb: [SystemZ] Allow configuring default CLANG_SYSTEMZ_ARCH (authored by uweigand).
[SystemZ] Allow configuring default CLANG_SYSTEMZ_ARCH
Mar 30 2020, 5:23 AM
uweigand closed D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH.
Mar 30 2020, 5:23 AM · Restricted Project, Restricted Project

Mar 27 2020

uweigand added a comment to D76124: [TTI] Remove getOperationCost.

Looking simply at the SystemZ test case change, for the icmp/[zs]ext case (fun1/fun2), we actually need three instructions (compare, load zero, conditional move), so the change seems reasonable.

Mar 27 2020, 7:37 AM · Restricted Project

Mar 26 2020

uweigand added a comment to D76771: [SystemZ] Improve foldMemoryOperandImpl: MS(G)RKC -> MS(G)C.

The more I think about it, the more it seems that the original check has always been somewhat questionable.

Mar 26 2020, 11:55 AM · Restricted Project
uweigand added a comment to D76624: [MSan] Add instrumentation for SystemZ.

Had a look at the vararg handling. Reviewed only the ABI-relevant parts.

Mar 26 2020, 9:11 AM · Restricted Project
uweigand committed rGdc37287320cc: [asan] Fix read_binary_name_regtest.c test dying with SIGPIPE (authored by iii).
[asan] Fix read_binary_name_regtest.c test dying with SIGPIPE
Mar 26 2020, 5:56 AM