Page MenuHomePhabricator

uweigand (Ulrich Weigand)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 14 2013, 11:48 AM (405 w, 4 d)

Recent Activity

Today

uweigand added a comment to D82862: [ThinLTO] Always parse module level inline asm with At&t dialect.

The motivation for my change was really just to make ThinLTO compiles work the same as non-ThinLTO ones.

Maybe the fact that -x86-asm-syntax=intel doesn't affect inline asm is a bug. I wasn't aware that Clang and GCC's -masm= flags behaved differently in that way, but that certainly suggests there's a problem here.

So I'm wondering, if I remove the above setAssemblerDialect line and revert this commit, we should have inline asm (both module-level and GNU function-leve) respect the target-selected asm dialect variant both for ThinLTO and non-ThinLTO, so they should match again. Would that also solve the problem you were originally tracking?

Not completely, because clang-cl sets -x86-asm-syntax=intel to enable intel-style asm in assembly listing output. We'd have to find another way of doing that without affecting the inline asm dialect.

Thu, Jan 21, 9:36 AM · Restricted Project, Restricted Project
uweigand committed rGb3a5abcb3696: [NFC][Doc] Mention SystemZ supports StackMap generation (authored by uweigand).
[NFC][Doc] Mention SystemZ supports StackMap generation
Thu, Jan 21, 9:31 AM
uweigand closed D95040: [NFC][Doc] Mention SystemZ supports StackMap generation.
Thu, Jan 21, 9:31 AM · Restricted Project

Yesterday

uweigand accepted D95040: [NFC][Doc] Mention SystemZ supports StackMap generation.

LGTM, thanks!

Wed, Jan 20, 6:23 AM · Restricted Project

Fri, Jan 15

uweigand added a comment to D82862: [ThinLTO] Always parse module level inline asm with At&t dialect.

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 Intel dialect, it has different representation in input, output, clobber etc. with GCC'.

Fri, Jan 15, 3:51 AM · Restricted Project, Restricted Project

Thu, Jan 14

uweigand added a comment to D94250: [SystemZ] Introducing assembler dialects for the Z backend.

Just a quick comment that I'm looking at this, but before approval I want to resolve the issues described in the comment in front of getMAIAssemblerDialect. See also discussion here: https://reviews.llvm.org/D82862

Thu, Jan 14, 7:17 AM · Restricted Project
uweigand added a comment to D82862: [ThinLTO] Always parse module level inline asm with At&t dialect.

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: https://lists.llvm.org/pipermail/llvm-dev/2020-November/146875.html

Thu, Jan 14, 7:15 AM · Restricted Project, Restricted Project

Wed, Jan 13

uweigand accepted D94383: [SystemZ] Don't crash with -misched-cutoff.

LGTM

Wed, Jan 13, 1:07 AM · Restricted Project

Tue, Jan 12

uweigand added a comment to D94383: [SystemZ] Don't crash with -misched-cutoff.

I don't think we need to bother with skipping advancing the hazard state. I believe the main point of the cutoff is to avoid combinatorial explosion where there are many instructions to schedule and at each step there are many candidates to consider. Advancing the hazard state doesn't consider candidates and is therefore just a linear pass over instructions.

Tue, Jan 12, 2:00 AM · Restricted Project

Mon, Jan 11

uweigand added a comment to D94383: [SystemZ] Don't crash with -misched-cutoff.

Could you elaborate why initPolicy is the correct place to clear the Available list? I'm wondering because the default implementation doesn't appear to do that either, it looks like common code only clears the list in the main "init" ...

Mon, Jan 11, 5:29 AM · Restricted Project

Dec 16 2020

uweigand accepted D93388: [Doc][SystemZ] Add Linux/SystemZ to Getting Started guide..

LGTM, thanks!

Dec 16 2020, 7:25 AM · Restricted Project

Dec 15 2020

uweigand committed rGebef92169ca5: [SystemZ] Remove most hard-coded R1D instances for sibcalls (authored by uweigand).
[SystemZ] Remove most hard-coded R1D instances for sibcalls
Dec 15 2020, 7:32 AM

Dec 14 2020

uweigand accepted D93171: [SystemZ] Improve handling of backchain offset.

This all looks good to me, the test case also looks fine (and the offset match what GCC is doing).

Dec 14 2020, 9:23 AM · Restricted Project

Dec 11 2020

uweigand accepted D92985: [SystemZTTIImpl::getMinPrefetchStride] Allow some non-prefetched mem accesses..

OK, I think this is fine then.

Dec 11 2020, 12:30 AM · Restricted Project
uweigand accepted D92803: [SystemZFrameLowering] Make sure R1 holding the backchain is not corrupted by probing .

Ah, OK . Looks good to me.

Dec 11 2020, 12:29 AM · Restricted Project

Dec 10 2020

uweigand accepted D92803: [SystemZFrameLowering] Make sure R1 holding the backchain is not corrupted by probing .

This suggestion wasn't for performance reasons, but correctness. In theory, when using the backchain, it should be updated on every change to %r15 so that %r15 at all times points to a valid backchain value. Otherwise, unwinding using the backchain will randomly fail when starting at some PC where the stack pointer has been updated but the backchain not yet written.

Dec 10 2020, 2:13 AM · Restricted Project
uweigand added a comment to D92985: [SystemZTTIImpl::getMinPrefetchStride] Allow some non-prefetched mem accesses..

It doesn't seem right to bail just for one non-prefetched access, so it seems reasonable to allow a relatively very small amount of non-prefetched instructions, to make the heuristic more stable. This patch suggests allowing 1 non-prefetched memory access per 32 prefetched ones. This handles LBM, and also gives two prefetches to imagick which however do not seem to play a role.

Dec 10 2020, 1:52 AM · Restricted Project

Dec 9 2020

uweigand added a comment to D92803: [SystemZFrameLowering] Make sure R1 holding the backchain is not corrupted by probing .

I think it still makes sense to have the backchain store local in inlineStackProbe.

Dec 9 2020, 10:20 AM · Restricted Project

Dec 8 2020

uweigand added a comment to D92803: [SystemZFrameLowering] Make sure R1 holding the backchain is not corrupted by probing .

I'm not sure I like the implicit assumptions the patch makes between inlineStackProbe and emitPrologue.

Dec 8 2020, 12:56 AM · Restricted Project

Nov 27 2020

uweigand accepted D92185: [SystemZ] Adding extra extended mnemonics for SystemZ target.

LGTM, thanks!

Nov 27 2020, 9:03 AM · Restricted Project
uweigand added a comment to D92185: [SystemZ] Adding extra extended mnemonics for SystemZ target.

This looks good to me. The only thing I'm wondering is whether we shouldn't complete the NOP set by adding a 6-byte "jgnop". (I guess this is called jlnop with HLASM, but in order to keep with the jg* naming scheme in GAS we probably ought to use jgnop here.)

Nov 27 2020, 1:41 AM · Restricted Project

Nov 24 2020

uweigand accepted D91962: Add support for STRICT_FSETCC promotion.

LGTM, thanks!

Nov 24 2020, 6:23 AM · Restricted Project
uweigand added a comment to D91962: Add support for STRICT_FSETCC promotion.

OK, I see. I guess the patch LGTM then with the two issues fixed.

Nov 24 2020, 5:48 AM · Restricted Project
uweigand added a comment to D91962: Add support for STRICT_FSETCC promotion.

See the two inline comments about STRICT_FSETCCS. I guess that shows this path is not really exercised right now -- maybe it would be a good idea to add some tests.

Nov 24 2020, 5:32 AM · Restricted Project

Nov 23 2020

uweigand added a comment to D91962: Add support for STRICT_FSETCC promotion.

Oh, any another thing: we might also want to handle STRICT_FSETCCS in addition to STRICT_FSETCC.

Nov 23 2020, 10:28 AM · Restricted Project
uweigand requested changes to D91962: Add support for STRICT_FSETCC promotion.
Nov 23 2020, 10:23 AM · Restricted Project

Nov 18 2020

uweigand accepted D91697: [SystemZ] Use ISD::ABS opcode during isel.

LGTM, thanks!

Nov 18 2020, 5:09 AM · Restricted Project

Nov 17 2020

uweigand added a comment to D91120: [DAGCombine][PowerPC] Convert negated abs to trivial arithmetic ops.

It seems strange that we would need to add a TLI hook to avoid regressing a target that has abs/nabs support in the ISA.
If I'm seeing it correctly, SystemZ relies on ISD::ABS being expanded and then pattern-matching the expansion back to lpgr for example. Could we change that target to declare ISD::ABS as legal and adjust the tablegen matching for it?

Nov 17 2020, 7:10 AM · Restricted Project

Nov 9 2020

uweigand added a comment to D90760: [InstCombiner] Make LibCallSimplifier add extension attribute to ldexp arg..

I'm not sure whether the call instruction must match the declaration w.r.t. extension attributes. In the IR generated by clang, the two always match:

Nov 9 2020, 7:50 AM · Restricted Project

Nov 4 2020

uweigand added a comment to D90760: [InstCombiner] Make LibCallSimplifier add extension attribute to ldexp arg..

This doesn't look quite right to me. ldexp has a signed "int" argument, so you always must use the "sext" sign extension attribute on that argument.

Nov 4 2020, 7:22 AM · Restricted Project

Oct 28 2020

uweigand committed rGa998cae0210f: [compiler-rt][SystemZ] Skip fuzzer/full-coverage.test (authored by uweigand).
[compiler-rt][SystemZ] Skip fuzzer/full-coverage.test
Oct 28 2020, 8:40 AM
uweigand added a comment to D90231: [GVN] Don't replace argument to @llvm.is.constant.*().

I think it makes general sense to not optimize the argument to @llvm.is.constant.

Oct 28 2020, 2:11 AM · Restricted Project

Oct 23 2020

uweigand accepted D90065: [SystemZ] Define MaxInstLength to have the value of 6..

Note: The python tests show up as UNSUPPORTED when I try to run them with llvm-lit. I could not find out how to enable them.

Oct 23 2020, 1:53 PM · Restricted Project

Oct 21 2020

uweigand accepted D89451: [SystemZ] Mark unsaved argument R6 as live throughout function.

See inline comment; otherwise this LGTM.

Oct 21 2020, 5:18 AM · Restricted Project

Oct 20 2020

uweigand committed rGc299f3555d77: [SystemZ] Fix disassembler crashes (authored by uweigand).
[SystemZ] Fix disassembler crashes
Oct 20 2020, 1:22 AM

Oct 16 2020

uweigand added inline comments to D89451: [SystemZ] Mark unsaved argument R6 as live throughout function.
Oct 16 2020, 5:30 PM · Restricted Project
uweigand added a comment to D87279: [clang] Fix handling of physical registers in inline assembly operands..

The problem seems to be with a tied earlyclobber operand:

asm("" : "+&r"(a));

Per the comment in InlineAsm::ConstraintInfo::Parse(), only output can be earlyclobber.

I am not sure if the earlyclobber ('&') should with this patch be removed from the added use of the register, or if this has to for some reason really be tied to the def?

Oct 16 2020, 5:32 AM · Restricted Project
uweigand added inline comments to D89451: [SystemZ] Mark unsaved argument R6 as live throughout function.
Oct 16 2020, 1:57 AM · Restricted Project

Oct 15 2020

uweigand updated subscribers of D89447: [MachineInstr] Add support for instructions with multiple memory operands..

@jonpa can you check whether the SystemZ test case you added still checks what it was intended to check here?

Oct 15 2020, 9:58 AM · Restricted Project

Oct 14 2020

uweigand accepted D89389: [SystemZ] Bugfix in SystemZVectorConstantInfo.

Strange that this wasn't noticed earlier. In any case, patch LGTM.

Oct 14 2020, 6:12 AM · Restricted Project

Oct 9 2020

uweigand accepted D89034: [SystemZ] Use LA instead of AGR in eliminateFrameIndex()..

LGTM, thanks!

Oct 9 2020, 2:05 AM · Restricted Project

Oct 8 2020

uweigand added inline comments to D89034: [SystemZ] Use LA instead of AGR in eliminateFrameIndex()..
Oct 8 2020, 7:49 AM · Restricted Project

Oct 6 2020

uweigand added a comment to D88888: [SystemZAsmParser] Treat VR128 separately in ParseDirectiveInsn()..

See inline comment. Otherwise LGTM.

Oct 6 2020, 5:25 AM · Restricted Project
uweigand added a comment to D88817: [llvm-readobj/elf] - Ignore the hash table when on EM_S390/EM_ALPHA platforms..

It looks like this will trigger a warning being emitted on every use of llvm-readobj on a EM_S390 object file, which is probably not what we want. (They all also contain .gnu.hash, so the fact that .hash is ignored is not anything that the user should be concerned about. -- This warning may just cause unnecessary confusion, or parse error with automated tools etc.)

I'd like to wait for opinions of @jhenderson/@MaskRay to confirm them are happy to silently ignore the .hash section on EM_S390/EM_ALPHA platforms. It might probably
be confusing to see a .hash section in an object and no output with --hash-table.

Oct 6 2020, 1:17 AM · Restricted Project

Oct 5 2020

uweigand added a comment to D88817: [llvm-readobj/elf] - Ignore the hash table when on EM_S390/EM_ALPHA platforms..

It looks like this will trigger a warning being emitted on every use of llvm-readobj on a EM_S390 object file, which is probably not what we want. (They all also contain .gnu.hash, so the fact that .hash is ignored is not anything that the user should be concerned about. -- This warning may just cause unnecessary confusion, or parse error with automated tools etc.)

Oct 5 2020, 4:47 AM · Restricted Project
uweigand accepted D88357: [SystemZ] Add support for .insn directives for vector instructions.

See the inline question about vector registers. This does not affect the current patch however, so this should be good to go in. LGTM.

Oct 5 2020, 4:44 AM · Restricted Project

Oct 2 2020

uweigand added inline comments to D88357: [SystemZ] Add support for .insn directives for vector instructions.
Oct 2 2020, 9:57 AM · Restricted Project

Oct 1 2020

uweigand added a comment to D88561: [llvm-readobj] - Fix possible crashes related to dumping gnu hash symbols..

I see. In this case this patch doesn't really address PR47681, but only replaces a crash with wrong output ... Still preferable to not crash, but of course we still need to fix the root cause (at the least, by ignoring the .hash section if the entry size doesn't match).

The fix for PR47681 (s390 specific) is independent from the crash (all platforms), so it should be fixed separatelly.
I'll am going to prepare a follow-up to fix the PR47681 after landing this.

Oct 1 2020, 4:07 AM · Restricted Project
uweigand added a comment to D88561: [llvm-readobj] - Fix possible crashes related to dumping gnu hash symbols..

I think the handling of the GNU hash table is actually already fully correct. The crash (or error message when using this patch) is simply a side effect of the fact that size of the DynSymRegion was clobbered here (near the end of ELFDumper<ELFT>::parseDynamicTable()):

DynSymRegion->Size = HashTable->nchain * DynSymRegion->EntSize;

This in turn is because HashTable->nchain (like all of HashTable) is just incorrect on s390x at the moment, as discussed in https://bugs.llvm.org/show_bug.cgi?id=47681

That is true for you executable. But this patch fixes a bit different case. Here I am creating the .gnu.hash table with a broken value in the buckets array.
Currently with the test case provided we crash an all platforms at the same place where your executable crashes too.
So with this patch we fix the crash, but not the reason of PR47681, which is s390x specific.

Oct 1 2020, 2:54 AM · Restricted Project
uweigand added inline comments to D88599: [SystemZ][ZOS] Porting pthread_t related functionality within libc++ to z/OS.
Oct 1 2020, 2:28 AM · Restricted Project
uweigand added inline comments to D88561: [llvm-readobj] - Fix possible crashes related to dumping gnu hash symbols..
Oct 1 2020, 1:38 AM · Restricted Project

Sep 30 2020

uweigand added a comment to D88561: [llvm-readobj] - Fix possible crashes related to dumping gnu hash symbols..

I think the handling of the GNU hash table is actually already fully correct. The crash (or error message when using this patch) is simply a side effect of the fact that size of the DynSymRegion was clobbered here (near the end of ELFDumper<ELFT>::parseDynamicTable()):

Sep 30 2020, 10:44 AM · Restricted Project

Sep 29 2020

uweigand requested review of D88479: [LLVM 11] Add SystemZ changes to release notes.
Sep 29 2020, 5:00 AM · Restricted Project
uweigand accepted D87510: [SystemZ] Don't emit PC-relative memory accesses to unaligned (packed) symbols..

LGTM now, thanks!

Sep 29 2020, 4:31 AM · Restricted Project

Sep 28 2020

uweigand added inline comments to D87510: [SystemZ] Don't emit PC-relative memory accesses to unaligned (packed) symbols..
Sep 28 2020, 9:26 AM · Restricted Project

Sep 25 2020

uweigand accepted D87988: [SystemZ] Optimize bcmp calls (PR47420).

LGTM as well.

Sep 25 2020, 7:10 AM · Restricted Project

Sep 15 2020

uweigand accepted D87390: [PowerPC] Pass nofpexcept flag to custom lowered constrained ops.

This LGTM, thanks!

Sep 15 2020, 9:39 AM · Restricted Project

Sep 11 2020

uweigand added inline comments to D87510: [SystemZ] Don't emit PC-relative memory accesses to unaligned (packed) symbols..
Sep 11 2020, 7:04 AM · Restricted Project

Sep 9 2020

uweigand committed rG1a25133bcdfe: [DAGCombine] Skip re-visiting EntryToken to avoid compile time explosion (authored by uweigand).
[DAGCombine] Skip re-visiting EntryToken to avoid compile time explosion
Sep 9 2020, 10:14 AM
uweigand closed D86963: [DAGCombine] Skip re-visiting EntryToken to avoid compile time explosion.
Sep 9 2020, 10:14 AM · Restricted Project
uweigand added a comment to D86963: [DAGCombine] Skip re-visiting EntryToken to avoid compile time explosion.

I'd like to make some progress on this soon, given that this still causes every LNT run on the SystemZ build bot to fail ...

Sep 9 2020, 9:03 AM · Restricted Project
uweigand added a comment to D86963: [DAGCombine] Skip re-visiting EntryToken to avoid compile time explosion.

Ping.

Sep 9 2020, 8:55 AM · Restricted Project

Sep 8 2020

uweigand accepted D87222: [PowerPC] [FPEnv] Disable strict FP mutation by default for PowerPC.

We're finally there, great :-) LGTM.

Sep 8 2020, 7:03 AM · Restricted Project
uweigand accepted D87220: [PowerPC] Fix STRICT_FRINT/STRICT_FNEARBYINT lowering.

Makes sense to me. As an aside, this code:

Sep 8 2020, 7:01 AM · Restricted Project, Restricted Project
uweigand accepted D86268: [DAGTypeLegalizer] Handle ZERO_EXTEND of promoted integer in WidenVecRes_Convert..

This LGTM now. Thanks!

Sep 8 2020, 6:03 AM · Restricted Project

Sep 4 2020

uweigand accepted D86605: [PowerPC] Expand constrained ppc_fp128 to i32 conversion.

LGTM now, thanks!

Sep 4 2020, 10:45 AM · Restricted Project
uweigand added a comment to D87130: [SelectionDAGBuilder] Remember to copy the FMF flags in visitBinary()..

I think this makes sense to me, but I want to point out that this partially reverts the commit https://reviews.llvm.org/D46854. I guess there may be other parts of this commit that should be reverted?

The other thing that was removed by that commit was the propagation of the NoNaNs flag from the experimental_vector_reduce_fmax (/fmin) intrinsic to the replacing VECREDUCE_FMAX (/FMIN) node, but I am not sure why...

Sep 4 2020, 9:52 AM · Restricted Project
uweigand added inline comments to D86605: [PowerPC] Expand constrained ppc_fp128 to i32 conversion.
Sep 4 2020, 9:46 AM · Restricted Project
uweigand added inline comments to D87115: [FPEnv][X86][SystemZ] Use different algorithms for i64->double uint_to_fp under strictfp to avoid producing -0.0 when rounding toward negative infinity.
Sep 4 2020, 9:44 AM · Restricted Project
uweigand added inline comments to D86605: [PowerPC] Expand constrained ppc_fp128 to i32 conversion.
Sep 4 2020, 3:52 AM · Restricted Project
uweigand added a comment to D87130: [SelectionDAGBuilder] Remember to copy the FMF flags in visitBinary()..

I think this makes sense to me, but I want to point out that this partially reverts the commit https://reviews.llvm.org/D46854. I guess there may be other parts of this commit that should be reverted?

Sep 4 2020, 3:39 AM · Restricted Project
uweigand added inline comments to D87115: [FPEnv][X86][SystemZ] Use different algorithms for i64->double uint_to_fp under strictfp to avoid producing -0.0 when rounding toward negative infinity.
Sep 4 2020, 3:32 AM · Restricted Project
uweigand added a comment to D86871: [SelectionDAG] Let NSW/NUW flags be cleared by default in call to getNode()..

I may be missing something, but as long as the memoization does a true intersection of the flags per this patch, there should not necessarily be anything broken, or?

Sep 4 2020, 3:16 AM · Restricted Project

Sep 2 2020

uweigand added a comment to D86871: [SelectionDAG] Let NSW/NUW flags be cleared by default in call to getNode()..

Thanks for the links, @spatel ! It seems to me that the change in "D46854 - changed code in SelectionDAGBuilder::visit()" is actually not correctly respecting the shared flags semantics. The node returned from getNodeForIRValue, while implementing the translation of this IR, might at the same be be used elsewhere (due to DAG node merging). If that other place does not allow FMF semantics, the flags in the DAG node can still be "undefined", and therefore the new code in "visit" will just replace the flags with a version appropriate only for this IR (which may allow FMF).

Am I missing something here? More generally, how can any after-the-fact setFlags ever be correct, given that the node might have already been merged?

That does seem wrong given the Flags.isDefined() check sitting within intersectWith(). I'm not seeing any regression test failures if we remove that entirely. So let's do that instead of only pushing it below the nsw/nuw intersects?

Sep 2 2020, 9:10 AM · Restricted Project
uweigand added a comment to D86605: [PowerPC] Expand constrained ppc_fp128 to i32 conversion.

The algorithm used in the unsigned case isn't strict-safe. However, this is exactly the same algorithm that is also used in TargetLowering::expandFP_TO_UINT, where is has been made properly strict-safe in the meantime.

Sep 2 2020, 8:24 AM · Restricted Project
uweigand added a comment to D86963: [DAGCombine] Skip re-visiting EntryToken to avoid compile time explosion.

Could you share the IR before DAGcombine that triggers the problem? DAGCombine already has limits in a few places that should prevent store merging from causing an explosion in compile-time. Would be good to know where this case slips through.

Sep 2 2020, 4:51 AM · Restricted Project

Sep 1 2020

uweigand requested review of D86963: [DAGCombine] Skip re-visiting EntryToken to avoid compile time explosion.
Sep 1 2020, 10:36 AM · Restricted Project
uweigand added a comment to D86871: [SelectionDAG] Let NSW/NUW flags be cleared by default in call to getNode()..

Thanks for the links, @spatel ! It seems to me that the change in "D46854 - changed code in SelectionDAGBuilder::visit()" is actually not correctly respecting the shared flags semantics. The node returned from getNodeForIRValue, while implementing the translation of this IR, might at the same be be used elsewhere (due to DAG node merging). If that other place does not allow FMF semantics, the flags in the DAG node can still be "undefined", and therefore the new code in "visit" will just replace the flags with a version appropriate only for this IR (which may allow FMF).

Sep 1 2020, 9:29 AM · Restricted Project
uweigand accepted D86595: [PowerPC] Handle STRICT_FSETCC(S) in more cases.

This LGTM now, thanks!

Sep 1 2020, 12:47 AM · Restricted Project

Aug 31 2020

uweigand added a comment to D86871: [SelectionDAG] Let NSW/NUW flags be cleared by default in call to getNode()..

Interesting. I agree that the flags should always be reset here. In fact, I'm questioning why we should have that "isDefined" logic in the first place. It seems to me that any node with "undefined" flags can only be used correctly if any setting of the flags would be semantically correct -- but then we may just as well use the all-zero setting of the flags anyway.

Aug 31 2020, 6:59 AM · Restricted Project

Aug 28 2020

uweigand added inline comments to D86268: [DAGTypeLegalizer] Handle ZERO_EXTEND of promoted integer in WidenVecRes_Convert..
Aug 28 2020, 6:17 AM · Restricted Project

Aug 26 2020

uweigand requested changes to D86595: [PowerPC] Handle STRICT_FSETCC(S) in more cases.
Aug 26 2020, 8:48 AM · Restricted Project

Aug 21 2020

uweigand accepted D81669: [PowerPC] Support constrained fp operation for scalar sitofp/uitofp.

OK, this now looks all good to me as well. Thanks!

Aug 21 2020, 5:03 AM · Restricted Project

Aug 20 2020

uweigand added a comment to D81669: [PowerPC] Support constrained fp operation for scalar sitofp/uitofp.

A few comments inline, otherwise this looks good.

Aug 20 2020, 8:47 AM · Restricted Project
uweigand accepted D81918: [PowerPC] Support lowering int-to-fp on ppc_fp128.

LGTM now. Thanks!

Aug 20 2020, 8:34 AM · Restricted Project

Aug 19 2020

uweigand accepted D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui.

This LGTM now. Thanks!

Aug 19 2020, 8:10 AM · Restricted Project

Aug 13 2020

uweigand added inline comments to D81918: [PowerPC] Support lowering int-to-fp on ppc_fp128.
Aug 13 2020, 6:00 AM · Restricted Project
uweigand added inline comments to D81537: [PowerPC] Support constrained fp operation for scalar fptosi/fptoui.
Aug 13 2020, 5:00 AM · Restricted Project

Aug 12 2020

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

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

Aug 12 2020, 11:43 AM · Restricted Project

Aug 10 2020

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?

Aug 10 2020, 10:27 AM · Restricted Project

Aug 4 2020

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.

Aug 4 2020, 5:50 AM · Restricted Project

Aug 3 2020

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

This LGTM now.

Aug 3 2020, 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.

Aug 3 2020, 3:40 AM · Restricted Project

Jul 30 2020

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

Sure, of course assuming the test still passes.

Jul 30 2020, 2:40 AM · Restricted Project

Jul 29 2020

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

LGTM for SystemZ as well.

Jul 29 2020, 10:38 AM · Restricted Project

Jul 24 2020

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

Jul 23 2020

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
Jul 23 2020, 6:35 AM

Jul 22 2020

uweigand accepted D84341: Implement __builtin_eh_return_data_regno for SystemZ.

LGTM with the format fixes.

Jul 22 2020, 10:29 AM · Restricted Project

Jul 21 2020

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

This LGTM now.

Jul 21 2020, 12:46 AM · Restricted Project