Page MenuHomePhabricator

uweigand (Ulrich Weigand)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 14 2013, 11:48 AM (449 w, 6 d)

Recent Activity

Tue, Nov 23

uweigand added a comment to D110267: [InlineAsm, SystemZ] Handle inline assembly address operands..

Finally getting back to this patch - sorry for the long delay.

Tue, Nov 23, 8:05 AM · Restricted Project

Mon, Nov 22

uweigand added a comment to D114354: [DAG] SimplifyDemandedBits - simplify rotl/rotr to shl/srl.

I don't understand the changes to the SystemZ test cases. The new behavior seems to be simply wrong?

Mon, Nov 22, 9:27 AM · Restricted Project

Fri, Nov 19

uweigand added a comment to D114194: [SystemZ] Add range checks for PC-relative fixups.

Not sure what to do with the testing: should we do something similar as a 'Large' folder as is already done for the branch relaxation tests?

Fri, Nov 19, 4:41 AM · Restricted Project

Tue, Nov 16

uweigand added a comment to D113960: PrologEpilogInserter: Use explicit control for scavenge slot placement.

SystemZ changes LGTM.

Tue, Nov 16, 12:45 AM · Restricted Project

Mon, Nov 15

uweigand updated subscribers of D113894: [x86/asm] Make variants work when converting at&t inline asm input to intel asm output.

Thanks @hans for the heads-up, I do indeed think this patch would break inline asm on z/OS.

Do you have any test coverage of z/OS? All tests pass with this as far as I can tell.

Mon, Nov 15, 9:17 AM · Restricted Project
uweigand added a comment to D113341: [SystemZ] Support symbolic displacements..

I tried this and it was simple enough - with just an increment in a single place in getDispOpValue(). However, encodeInstruction() is an overriden const function, so clearing MemOpsEmitted wass not permitted. Then it occured to me that if we really can make this broad assumption, we could just check the size of the Fixups vector and that way differentiate between the first and second fixup.

Mon, Nov 15, 9:12 AM · Restricted Project
uweigand added a comment to D113894: [x86/asm] Make variants work when converting at&t inline asm input to intel asm output.

Thanks @hans for the heads-up, I do indeed think this patch would break inline asm on z/OS.

Mon, Nov 15, 8:56 AM · Restricted Project
uweigand added inline comments to D112004: [SystemZ] Improve codegen for memset.
Mon, Nov 15, 6:10 AM · Restricted Project
uweigand added a comment to D112004: [SystemZ] Improve codegen for memset.

LGTM, just a few minor nits inline. We also should do a performance validation run before committing this.

Mon, Nov 15, 4:41 AM · Restricted Project

Fri, Nov 12

uweigand added a comment to D113341: [SystemZ] Support symbolic displacements..
  • Mapping of MI:OperandIndex to fixup byte offset: I added a TSFlags bit 'MemMem' which is checked for in getDispOpValue(). From this flag it is known that there are two memory operands, with displacements starting at bit 20 and 36. For the matter of choosing which one of these are in question in getDispOpValue(), based on the OpNum, I was not sure if this could be hard coded as well in the formats file, or if it would be simpler to check this at compile time the way that is currently done..?
Fri, Nov 12, 8:29 AM · Restricted Project

Tue, Nov 9

uweigand added a comment to D113493: [CodeGen] Update LiveIntervals in TargetInstrInfo::convertToThreeAddress.

SystemZ changes LGTM - I guess we're always in the "SingleInst" case.

Tue, Nov 9, 10:20 AM · Restricted Project

Oct 27 2021

uweigand added a comment to D112004: [SystemZ] Improve codegen for memset.

I think it would be a bit simpler and clearer to handle the Src vs. Dest address special case for memset in a single place at the beginning. Something along the lines of:

Oct 27 2021, 6:16 AM · Restricted Project

Oct 26 2021

uweigand accepted D112065: [SystemZ] Improvement of emitMemMemWrapper().

This looks like a nice improvement. LGTM.

Oct 26 2021, 6:41 AM · Restricted Project

Oct 21 2021

uweigand accepted D112172: [SystemZ][z/OS] Additional test coverage for validating dialect instructions for SystemZ.

LGTM, thanks!

Oct 21 2021, 3:40 AM · Restricted Project

Oct 13 2021

uweigand accepted D111733: [SystemZ] Reapply memcmp and memcpy patches.

LGTM, thanks!

Oct 13 2021, 11:44 AM · Restricted Project
uweigand accepted D111729: [SystemZ] Bugfix and refactorization of mem-mem operations.

LGTM, thanks!

Oct 13 2021, 11:41 AM · Restricted Project
uweigand accepted D111653: [z/OS] Implement save of non-volatile registers on z/OS XPLINK.

LGTM, thanks!

Oct 13 2021, 5:11 AM · Restricted Project
uweigand accepted D111662: [SystemZ][z/OS] Initial implementation for lowerCall on z/OS.

LGTM, thanks!

Oct 13 2021, 5:10 AM · Restricted Project

Oct 11 2021

uweigand committed rZORGdfd41aa8267c: mlir-s390x-linux: collapse requests (authored by uweigand).
mlir-s390x-linux: collapse requests
Oct 11 2021, 11:19 AM

Oct 8 2021

uweigand added a comment to D111437: [SystemZ/z/OS] Implement GOFF writer for empty files.

The SystemZ back-end changes LGTM.

Oct 8 2021, 10:02 AM · Restricted Project

Oct 1 2021

uweigand accepted D110730: [SystemZ][z/OS] Introduce initial support for GOFF asm parser.

LGTM as well.

Oct 1 2021, 12:58 AM · Restricted Project

Sep 30 2021

uweigand added inline comments to D110730: [SystemZ][z/OS] Introduce initial support for GOFF asm parser.
Sep 30 2021, 11:00 AM · Restricted Project

Sep 29 2021

uweigand committed rZORG60bd26817244: Add mlir builder for s390x (authored by uweigand).
Add mlir builder for s390x
Sep 29 2021, 12:52 PM
uweigand closed D109745: Add mlir builder for s390x.
Sep 29 2021, 12:52 PM
uweigand added inline comments to D110730: [SystemZ][z/OS] Introduce initial support for GOFF asm parser.
Sep 29 2021, 10:48 AM · Restricted Project

Sep 24 2021

uweigand accepted D110077: [SystemZ][z/OS] Introduce the GOFFMCAsmInfo Interface for z/OS.

LGTM

Sep 24 2021, 12:26 PM · Restricted Project
uweigand accepted D109598: [SystemZ] NFC: Remove unused intrinsic template arg 'name'.

LGTM, thanks!

Sep 24 2021, 4:26 AM · Restricted Project

Sep 23 2021

uweigand accepted D110346: [SystemZ] Implement ISD::BITCAST for fp128 -> i128..

LGTM, thanks!

Sep 23 2021, 10:42 AM · Restricted Project

Sep 21 2021

uweigand added a comment to D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout.
  • 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)
Sep 21 2021, 11:51 AM · Restricted Project, Restricted Project
uweigand accepted D109513: [AsmPrinter, SystemZ] Allow target to add instructions before section is ended..

LGTM now, thanks!

Sep 21 2021, 4:35 AM · Restricted Project

Sep 20 2021

uweigand added a comment to D91944: OpenMP 5.0 metadirective.

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 by specifying the triple. (https://github.com/llvm/llvm-project/commit/3679d2001c87f37101e7f20c646b21e97d8a0867)

Sep 20 2021, 12:00 PM · Restricted Project, Restricted Project, Restricted Project
uweigand added a comment to D109513: [AsmPrinter, SystemZ] Allow target to add instructions before section is ended..

Two minor nits inline, otherwise I agree we should go with that approach now.

Sep 20 2021, 4:42 AM · Restricted Project
uweigand added a comment to D91944: OpenMP 5.0 metadirective.

Looks like this was committed again, breaking the SystemZ build bots once again:
https://lab.llvm.org/buildbot/#/builders/94/builds/5661

Sep 20 2021, 3:47 AM · Restricted Project, Restricted Project, Restricted Project
uweigand added a comment to D109657: [SystemZ] Accept plain register name where an address is expected..

I believe %reg should be treated equivalently to 0(%reg) for any type of address, including those with length or vector index, so this should be fine. However, it would be good to validate that behavior against GAS, and also add a few test cases for instructions with those address types.

For addresses with a Length field, it seems GAS does require the full address with the explicit displacement as well:

        xc      0(32,%r2), 0(%r2)
        xc      0(32,%r2), %r2
#       xc      (32,%r2), %r2    # not accepted by GAS
#       xc      32,%r2, %r2      # not accepted by GAS
Sep 20 2021, 3:16 AM · Restricted Project

Sep 16 2021

uweigand added inline comments to D109850: Implement SystemZIselLowering::hasAndNot.
Sep 16 2021, 7:13 AM · Restricted Project

Sep 15 2021

uweigand accepted D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout.

This LGTM now.

Sep 15 2021, 10:17 AM · Restricted Project, Restricted Project
uweigand added inline comments to D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout.
Sep 15 2021, 9:52 AM · Restricted Project, Restricted Project
uweigand added a comment to D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout.

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?

At a future point, we will be changing the local and global prefixes. At that point we would still need a separate MM_GOFF field. I feel it would be a bit better to enforce it from now itself.

Sep 15 2021, 1:47 AM · Restricted Project, Restricted Project

Sep 14 2021

uweigand added a comment to D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout.

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?

Sep 14 2021, 7:27 AM · Restricted Project, Restricted Project
uweigand requested review of D109745: Add mlir builder for s390x.
Sep 14 2021, 1:05 AM
uweigand accepted D109660: [SystemZ] Recognize .machine directive in parser..

SystemZTargetStreamer added with derived and registered SystemZTargetAsmStreamer and SystemZTargetELFStreamer classes. The object TargetStreamer does not output anything in emitMachine(), which I don't think it should, or?

Sep 14 2021, 12:29 AM · Restricted Project

Sep 13 2021

uweigand accepted D109702: [NFC] Replace hard-coded usages of SystemZ::R15D with SpecialRegisters API.

LGTM.

Sep 13 2021, 9:45 AM · Restricted Project
uweigand added a comment to D109660: [SystemZ] Recognize .machine directive in parser..

Does such a directive need to be output again from the parser in case output is back into textual form (filetype=asm)? This is currently not done by the patch - I see that these are dropped after running GAS | objdump... (object format). I guess in that case either MCTargetStreamer is needed, or possibly some RawText emission..?

Sep 13 2021, 4:17 AM · Restricted Project
uweigand added a comment to D109657: [SystemZ] Accept plain register name where an address is expected..

This patch is not checking the expected kind of address (MemKind) or HasLength / HasVectorIndex - would that be better?

Sep 13 2021, 4:02 AM · Restricted Project

Sep 10 2021

uweigand added a comment to D109513: [AsmPrinter, SystemZ] Allow target to add instructions before section is ended..

Ah, I see, we've have to implement a MCTargetStreamer first ... In any case, I don't have a strong opinion on this, if the common code change is acceptable, that's of course fine with me. A back-end only solution using a MCTargetStreamer would also be OK.

Sep 10 2021, 11:03 AM · Restricted Project
uweigand added a comment to D109513: [AsmPrinter, SystemZ] Allow target to add instructions before section is ended..

do you think it'd make sense to merge emitConstantPools into emitSectionEndings?

How about moving emitConstantPools() into the default implementation of emitSectionEndings() like this..?

Hmm ... that makes me wonder: can we instead just use the existing emitConstantPools in the SystemZ back end to output the EXRL instructions? They are in effect a form of constants ... That shouldn't require any common code changes then.

I read in the original bug report: "...adding the XC instructions in some other text section out of the eyes of debug is not a good option for z/OS (details excluded)". Therefore I thought they better be placed in the text section...

Sep 10 2021, 5:17 AM · Restricted Project
uweigand added a comment to D109513: [AsmPrinter, SystemZ] Allow target to add instructions before section is ended..

do you think it'd make sense to merge emitConstantPools into emitSectionEndings?

How about moving emitConstantPools() into the default implementation of emitSectionEndings() like this..?

Sep 10 2021, 3:17 AM · Restricted Project

Sep 9 2021

uweigand accepted D108777: [SystemZ] [NFC] Add SystemZELFFrameLowering and SystemZXPLINKFrameLowering classes..

Formatting looks good to me now as well. Still LGTM.

Sep 9 2021, 3:13 AM · Restricted Project

Sep 7 2021

uweigand added a comment to D109362: [SystemZ][z/OS] Add GOFF Support to the DataLayout.

SystemZ specific parts LGTM, but it would be good to have someone else review the common code / IR changes as well.

Sep 7 2021, 7:37 AM · Restricted Project, Restricted Project
uweigand added a comment to D107380: [SystemZ] Implement memcmp with variable length with CLC..

This LGTM, but it would be good to see some full benchmark results as well.

Sep 7 2021, 7:30 AM · Restricted Project

Sep 6 2021

uweigand accepted D109291: [SelectionDAGBuilder] Bugfix in visitInlineAsm().

As you discuss in the bugzilla, this seems a good compromise between using the (larger) RC associated with the type when available, and falling back to the (minimal) RC associated with the physreg if we have a "weird" type. LGTM.

Sep 6 2021, 2:59 AM · Restricted Project

Aug 27 2021

uweigand accepted D108777: [SystemZ] [NFC] Add SystemZELFFrameLowering and SystemZXPLINKFrameLowering classes..

LGTM

Aug 27 2021, 8:53 AM · Restricted Project

Aug 25 2021

uweigand accepted D108639: [SystemZ] [NFC] Replace SpecialRegisters field with a unique_ptr instead of a raw pointer..

LGTM

Aug 25 2021, 1:16 AM · Restricted Project

Jul 27 2021

uweigand added a comment to D105067: [SystemZ] Emit .gnu_attribute for an externally visible vector abi..

I tried now to do it in LowerCall(), but as I did not find any way to set a flag at Module scope

Jul 27 2021, 8:18 AM · Restricted Project
uweigand added a comment to D106874: [SystemZ] Implement memcpy with variable length with MVC.

This looks good to me, but we should verify the performance using a full run.

Jul 27 2021, 8:15 AM · Restricted Project
uweigand added a comment to D106782: [llvm-objcopy] Drop GRP_COMDAT if the group signature is localized.

Looks like this caused a buildbot failure on s390x:
https://lab.llvm.org/buildbot/#/builders/94/builds/5110

A latent problem triggered by the change. Fixed by c5d8bd5a35cbd325c6ccd42afa91bad06d261f07

Jul 27 2021, 1:05 AM · Restricted Project

Jul 26 2021

uweigand added a comment to D106782: [llvm-objcopy] Drop GRP_COMDAT if the group signature is localized.

Looks like this caused a buildbot failure on s390x:
https://lab.llvm.org/buildbot/#/builders/94/builds/5110

Jul 26 2021, 2:47 PM · Restricted Project
uweigand committed rG8cd8120a7b5d: [SystemZ] Add support for new cpu architecture - arch14 (authored by uweigand).
[SystemZ] Add support for new cpu architecture - arch14
Jul 26 2021, 7:58 AM

Jul 20 2021

uweigand committed rGe04c05e8230e: [SystemZ] Fix invalid assumption in getCPUNameFromS390Model (authored by uweigand).
[SystemZ] Fix invalid assumption in getCPUNameFromS390Model
Jul 20 2021, 4:40 AM

Jul 14 2021

uweigand added a comment to D105629: [TSan] Add SystemZ support.

The SystemZ specific changes all look good to me now, thanks!

Jul 14 2021, 5:18 AM · Restricted Project, Restricted Project, Restricted Project

Jul 13 2021

uweigand added inline comments to D105629: [TSan] Add SystemZ support.
Jul 13 2021, 3:33 AM · Restricted Project, Restricted Project, Restricted Project
uweigand added inline comments to D105629: [TSan] Add SystemZ support.
Jul 13 2021, 3:23 AM · Restricted Project, Restricted Project, Restricted Project
uweigand added a comment to D105629: [TSan] Add SystemZ support.

See inline comments ... otherwise the SystemZ platform-specific parts look good to me.

Jul 13 2021, 2:21 AM · Restricted Project, Restricted Project, Restricted Project

Jul 12 2021

uweigand accepted D105757: [SystemZ] Bugfix for the 'N' code for inline asm operand..

LGTM, thanks!

Jul 12 2021, 1:55 AM · Restricted Project

Jul 6 2021

uweigand accepted D105502: [SystemZ] Support the 'N' code for the odd register in inline-asm..

LGTM, thanks!

Jul 6 2021, 10:41 AM · Restricted Project
uweigand added a comment to D105067: [SystemZ] Emit .gnu_attribute for an externally visible vector abi..

I don't think this implementation of vararg support is quite complete. On the caller side, you check for variable arguments to any direct function with a prototype. However, this misses the case where an *indirect* function call (using a function pointer) has variable arguments.

This also completely misses the *callee* side: every function that takes variable arguments and uses va_arg with a vector type is also dependent on the vector ABI.

Patch updated to address these cases as well.

Jul 6 2021, 5:04 AM · Restricted Project
uweigand accepted D103865: [SystemZ] Generate XC loop for memset 0 of variable length..

See the minor comment inside. Otherwise, this LGTM now. Thanks!

Jul 6 2021, 4:52 AM · Restricted Project
uweigand added a comment to D104765: [DAG] Reassociate Add with Or.

Makes sense to me as well. The SystemZ changes are fine.

Jul 6 2021, 1:38 AM · Restricted Project

Jul 1 2021

uweigand added a comment to D103865: [SystemZ] Generate XC loop for memset 0 of variable length..

Updated per review.

I was thinking of just using emitInt16 for the three words, that will show as .word in the textual assembler output, which may not be pretty but is certainly correct. (You could in addition use an emitRawComment to print a textual representation to make the file more readable).

Ah, ok, I did not realize at the time I could just print the instruction as numbers... I tried this instead now, since we don't really want to have a generic subtarget around. Is there any reason to print 3 x Int16? Would it be better to print it as one (or three) hex if possible?

Jul 1 2021, 1:59 AM · Restricted Project

Jun 30 2021

uweigand added a comment to D103865: [SystemZ] Generate XC loop for memset 0 of variable length..
Jun 30 2021, 2:37 AM · Restricted Project

Jun 29 2021

uweigand added inline comments to D105067: [SystemZ] Emit .gnu_attribute for an externally visible vector abi..
Jun 29 2021, 1:45 PM · Restricted Project

Jun 28 2021

uweigand added a comment to D103865: [SystemZ] Generate XC loop for memset 0 of variable length..

I updated the patch to create the length-1 and trip count on the DAG instead.

Jun 28 2021, 11:00 AM · Restricted Project
uweigand added a comment to D102894: [MCStreamer] Move emission of attributes section into MCELFStreamer.

The SystemZ part now looks good to me, except for the point made inline. It still would be good if the rest of the patch could be looked at by someone familiar with ARM. (Maybe it would make sense to split the patch two parts?)

Jun 28 2021, 9:59 AM · Restricted Project
uweigand accepted D104869: [AsmParser][SystemZ][z/OS] Fix hanging scenario in HLASMAsmParser class.

LGTM.

Jun 28 2021, 9:44 AM · Restricted Project

Jun 25 2021

uweigand committed rGb2674670f264: [SystemZ] Add support for .reloc assembler directive (authored by uweigand).
[SystemZ] Add support for .reloc assembler directive
Jun 25 2021, 12:51 PM

Jun 24 2021

uweigand accepted D104715: [AsmParser][SystemZ][z/OS] Support for emitting labels in upper case.

LGTM

Jun 24 2021, 9:27 AM · Restricted Project

Jun 22 2021

uweigand added a comment to D103865: [SystemZ] Generate XC loop for memset 0 of variable length..

One thing that is odd is the extra always-zero parameter to the LoopVarLen insns. This really should be removed.

Done - it was easy to remove on the MachineInstr, but on the SystemZISD node it seems reasonable to keep a dummy zero there and reuse the existing opcode, or?

Jun 22 2021, 6:07 AM · Restricted Project

Jun 18 2021

uweigand added a comment to D103865: [SystemZ] Generate XC loop for memset 0 of variable length..

This looks pretty good to me. One thing that is odd is the extra always-zero parameter to the LoopVarLen insns. This really should be removed.

Jun 18 2021, 5:59 AM · Restricted Project

Jun 7 2021

uweigand added a comment to D102894: [MCStreamer] Move emission of attributes section into MCELFStreamer.

I haven't looked at the code in detail yet, but just a couple of general comments:

  • The GNU attribute support should probably just go into the generic ELF target streamer. This concept is supported in principle on all ELF targets. (That might also avoid the need for multiple inheritance.)
  • If we support the GNU attribute generally in the ELF streamer, then we should also support it generally in the asm streamer (and then also the common asm parser, I guess)
  • ASM parser support for .gnu_attribute should support any tag&value just like GAS does.
Jun 7 2021, 6:44 AM · Restricted Project

Jun 2 2021

uweigand added a comment to D102046: [sanitizer] Fall back to fast unwinder.

I have implemented that requires for Arm in https://reviews.llvm.org/D103512.

Perhaps the same could apply to s390x?

Jun 2 2021, 6:09 AM · Restricted Project

May 31 2021

uweigand accepted D103343: [SystemZ][z/OS] Stricter condition for HLASM class instantiation.

LGTM, thanks!

May 31 2021, 9:07 AM · Restricted Project
uweigand added a comment to D102615: [LoopDeletion] Break backedge if we can prove that the loop is exited on 1st iteration (try 3).

Oh I see... That's nasty. In this case, could you please help me to reproduce it? I wasn't able to find the sources of this test in LLVM repository. IR dump before Loop Deletion would work too.

May 31 2021, 9:06 AM · Restricted Project
uweigand accepted D103320: [AsmParser][SystemZ][z/OS] Introducing HLASM Parser support to AsmParser - Part 2.

LGTM.

May 31 2021, 7:56 AM · Restricted Project
uweigand added a comment to D103343: [SystemZ][z/OS] Stricter condition for HLASM class instantiation.

Maybe it would be clearer to be explicit and write:

if (C.getTargetTriple().isSystemZ() && C.getTargetTriple().isOSzOS())

This should have the same effect

Yes, that would be the same behaviour.

but avoid the implicit assumptions.

Are we making an implicit assumption? In the getDefaultFormat function in Triple.cpp, we have:

case Triple::systemz:
  if (T.isOSzOS())
    return Triple::GOFF;

GOFF is only set when the arch is systemz and the os is zos.

May 31 2021, 7:55 AM · Restricted Project
uweigand added a comment to D102894: [MCStreamer] Move emission of attributes section into MCELFStreamer.

Patch updated - in progress.

This is probably not even necessary, since even a pointer to a vector would still trigger the emission of the gnu_attribute...
Yes, indeed. The alignment of 128-bit vector types is different between the two ABIs, which means that any use of such types, even via pointers or as struct elements, will trigger ABI differences.

Aha... I removed the function attribute emission from clang and now do all the work in SystemZAsmPrinter instead. Added a check for actually passed arguments to a vararg function.

For an example of how to handle such things, you might look at emitLocalEntry in the PowerPC target. You'll need a virtual function in TargetStreamer.h that can be called by the AsmPrinter.cpp code. Then there need to be two implementations of the virtual function, one for emitting assembler text, and one when directly emitting object files. Finally, you'll need to handle reading the directive in assembler code in the AsmParser.

I did an attempt at this except for the parser part, which seems to work.

May 31 2021, 6:46 AM · Restricted Project
uweigand added a comment to D102046: [sanitizer] Fall back to fast unwinder.

Ah wait: the test case uses -fno-asynchronous-unwind-tables ! This explains why unwind cannot work - on s390x the ABI requires .eh_frame everywhere, and there is no alternative unwind mechanism via a frame pointer. The "fast unwinder" instead uses the (deprecated) "backchain" mechanism, but that is off by default and requires use of the -mbackchain option. That option is actually set by the various .lit.cfg files, but I guess the special invocation in this test case ignores those flags?

May 31 2021, 6:19 AM · Restricted Project
uweigand added a comment to D102046: [sanitizer] Fall back to fast unwinder.

How about s/A<10>()/A<7>()/? Does that pass on s390x?

This passes, yes.

Can we make that change in mainline? I'd really like to get our build bots green again ...

Pushed 3a678fe3e29fe48d9b4efc936edd2ac43c720bae

May 31 2021, 6:00 AM · Restricted Project
uweigand added a comment to D103343: [SystemZ][z/OS] Stricter condition for HLASM class instantiation.

Maybe it would be clearer to be explicit and write:

May 31 2021, 2:12 AM · Restricted Project
uweigand added a comment to D102615: [LoopDeletion] Break backedge if we can prove that the loop is exited on 1st iteration (try 3).

Looking at the assembly, only a single function shows any difference between the two versions: "interp" from MultiSource/Benchmarks/MallocBench/gs/interp.c.

May 31 2021, 1:29 AM · Restricted Project
uweigand added a comment to D102615: [LoopDeletion] Break backedge if we can prove that the loop is exited on 1st iteration (try 3).

The last failure also looks like a timeout, I don't see any indication of miscompile:

FAIL: test-suite :: MultiSource/Benchmarks/MallocBench/gs/gs.test (73 of 2021)
******************** TEST 'test-suite :: MultiSource/Benchmarks/MallocBench/gs/gs.test' FAILED ********************

/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/tools/timeit-target --limit-core 0 --limit-cpu 7200 --timeout 7200 --limit-file-size 104857600 --limit-rss-size 838860800 --append-exitstatus --redirect-output /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/MultiSource/Benchmarks/MallocBench/gs/Output/gs.test.out --redirect-input /dev/null --chdir /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/MultiSource/Benchmarks/MallocBench/gs --summary /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/MultiSource/Benchmarks/MallocBench/gs/Output/gs.test.time /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/MultiSource/Benchmarks/MallocBench/gs/gs -DNODISPLAY INPUT/large.ps
cd /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/MultiSource/Benchmarks/MallocBench/gs ; /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/tools/fpcmp-target /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/MultiSource/Benchmarks/MallocBench/gs/Output/gs.test.out gs.reference_output

/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/sandbox/build/tools/fpcmp-target: FP Comparison failed, not a numeric difference between '
' and 'g'

@uweigand could you please confirm that the output like this could have been a result of timeout?

May 31 2021, 12:47 AM · Restricted Project

May 29 2021

uweigand added a comment to D101806: [TargetLowering] Only inspect attributes in the arguments for ArgListEntry.

Unfortunately, I now see this patch also caused another regression, this time in the s390x multistage builder. Again, this was initially hidden by other regressions; now that these are resolved, you can see the problems here (https://lab.llvm.org/buildbot/#/builders/8/builds/1277):
FAIL: LLVM-Unit::FixedPoint.FixedToFloat
FAIL: LLVM-Unit::FixedPoint.FloatToFixed
FAIL: Clang::fixed_point_compound.c
FAIL: Clang::fixed_point_conversions.c
FAIL: Clang::fixed_point_conversions_half.c
FAIL: Clang::fixed_point_conversions_const.c

May 29 2021, 9:12 AM · Restricted Project
uweigand committed rGc123c178b26e: [SystemZ] Set getExtendForAtomicOps to ISD::ANY_EXTEND (authored by uweigand).
[SystemZ] Set getExtendForAtomicOps to ISD::ANY_EXTEND
May 29 2021, 3:17 AM

May 28 2021

uweigand added a comment to D102615: [LoopDeletion] Break backedge if we can prove that the loop is exited on 1st iteration (try 3).

This commit also introduced a regression in the s390x-lnt buildbot due to an apparent miscompilation of the test-suite::gs.test test case:
https://lab.llvm.org/buildbot/#/builders/45/builds/3264

May 28 2021, 6:45 AM · Restricted Project
uweigand added a comment to D101806: [TargetLowering] Only inspect attributes in the arguments for ArgListEntry.

Hmm, the https://reviews.llvm.org/D103288 commit did fix the problem on s390x again. Thanks!

May 28 2021, 3:52 AM · Restricted Project

May 27 2021

uweigand added a comment to D101806: [TargetLowering] Only inspect attributes in the arguments for ArgListEntry.

I looks like this patch, when committed as 1c7f32334d4becc725b9025fd32291a0e5729acd, caused two additional build bot failures on s390x:
https://lab.llvm.org/buildbot/#/builders/94/builds/3829
FAIL: libFuzzer:: msan.test
FAIL: libFuzzer:: swap-cmp.test
This was unfortunately hidden by other failures at the time, so I didn't notice earlier.

May 27 2021, 9:42 AM · Restricted Project

May 26 2021

uweigand accepted D103111: [SystemZ][z/OS] Enable the AllowAtInName attribute for the HLASM dialect.

LGTM.

May 26 2021, 3:10 AM · Restricted Project
uweigand accepted D100788: [SystemZ] Support i128 inline asm operands.

This version LGTM now. Thanks!

May 26 2021, 3:09 AM · Restricted Project
uweigand added a comment to D103077: [DAGCombine] Poison-prove scalarizeExtractedVectorLoad..

Adjust failing SystemZ test. It would be great if someone familiar with SystemZ could take a look to double-check this makes sense. Perhaps @jonpa?

Yes, this looks like a correct test update to me: the changed immediate operand reflects that now only two shifted bits are inserted into %r1 (instead of all 32).

I wonder what the rationale behind this patch is: if the instruction is poison to begin with, then the program is broken and I don't understand how changing the argument value can change that..?

Adding @uweigand as well just in case...

May 26 2021, 3:05 AM · Restricted Project

May 25 2021

uweigand accepted D103091: [SystemZ][z/OS] Validate symbol names for z/OS for printing without quotes.

LGTM, thanks.

May 25 2021, 8:52 AM · Restricted Project
uweigand added a comment to D102894: [MCStreamer] Move emission of attributes section into MCELFStreamer.

Function return value / parameters: A C function with a vector parameter will be lowered by Clang differently depending on the subtarget vector support. On z13 it may be a <4 x i32>, while on zEC12 it is instead <4 x i32>*. When running instruction selection there would be then no way of telling if <4 x i32>* was a pointer argument or if it was a software ABI vector variable argument. Therefore a function attribute is emitted for each function "has-vector-arg" where as applicable. I guess the alternative might be to actually do this during instruction selection while considering the subtarget vector support presence - if using clang (and not llc), this might be enough. (E.g. if subtarget has vector support, then <4 x i32>* is really a pointer...)

May 25 2021, 3:46 AM · Restricted Project