Page MenuHomePhabricator

nemanjai (Nemanja Ivanovic)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 23 2015, 9:38 AM (312 w, 1 d)

Recent Activity

Thu, Jan 14

nemanjai requested changes to D94711: [PowerPC] Fix the check for the instruction using FRSP/XSRSP output register .

Test case is missing. Please add it.

Thu, Jan 14, 12:37 PM · Restricted Project
nemanjai accepted D92080: [Clang] Mutate long-double math builtins into f128 under IEEE-quad.

LGTM.
I think it would be useful to run a functional test with a toolchain that has these library functions. That shouldn't gate this change though.

Thu, Jan 14, 4:51 AM · Restricted Project
nemanjai added a comment to D94670: [DebugInfo][NFC] add a new DIE type to represent label + offset.

Added Eric as the owner of debug info.

Thu, Jan 14, 4:43 AM · Restricted Project
nemanjai added a reviewer for D94670: [DebugInfo][NFC] add a new DIE type to represent label + offset: echristo.
Thu, Jan 14, 4:43 AM · Restricted Project
nemanjai added a comment to D94668: [debug-info] [NFC] add isa support for MCStreamer.

Added Eric as the owner of debug info.

Thu, Jan 14, 4:43 AM · Restricted Project
nemanjai added a reviewer for D94668: [debug-info] [NFC] add isa support for MCStreamer: echristo.
Thu, Jan 14, 4:42 AM · Restricted Project

Wed, Jan 13

nemanjai requested changes to D90173: [PowerPC] Exploit splat instruction xxsplti32dx in Power10.

This is still incorrect. The indices for the hi/low words are backwards. You can easily demonstrate this with a test case such as:

a.c:
vector double test() { return (vector double)4.23422e12; }
Wed, Jan 13, 8:32 PM · Restricted Project, Restricted Project, Restricted Project
nemanjai added a comment to D94054: [PowerPC] Try to fold sqrt/sdiv test results with the branch..

Thanks for addressing my comments and I'm sorry I didn't get a chance to re-review prior to commit. Looks good now.

Wed, Jan 13, 7:36 PM · Restricted Project
nemanjai added a comment to D94467: [PowerPC] Use mtvsrdd+vpku instructions to optimize build_vector.

If all the values are in GPR's, the code produced with this patch:

mtvsrdd 34, 4, 3
mtvsrdd 35, 6, 5
vpkudum 2, 3, 2
mtvsrdd 35, 8, 7
mtvsrdd 36, 10, 9
vpkudum 3, 4, 3
vpkuwum 2, 3, 2

is certainly better than the naive code we currently produce. But I don't think we should be doing the merging/packing in the vector domain because (at least on P9) we get half the dispatch width and the permute operations potentially have a higher latency. Furthermore, there is a potential of increasing vector register pressure with this approach which is probably not ideal. I think that for the basic case (where all values are in GPR's) we should simply add a pattern in the .td file that does something like this (similar to what we did for the wider elements):

rlwimi 3, 4, ...  # merge r3 and r4
rlwimi 5, 6, ...  # merge r5 and r6
rlwimi 7, 8, ...  # merge r7 and r8
rlwimi 9, 10, ... # merge r9 and r10
rldimi 3, 5, ...  # merge r3, r4, r5, r6
rldimi 7, 9, ...  # merge r7, r8, r9, r10
mtvsrdd 34, 3, 7

For 32-bit mode, we can't really do the merging to doublewords in GPR's but I think they can be moved to VSR's after the word merges and then merged with a single vpkuwum.

Wed, Jan 13, 4:58 PM · Restricted Project

Tue, Jan 12

nemanjai added a comment to D94449: [PowerPC][NFCI] PassSubtarget to ASMWriter.

Please mention the commit that added this support initially (and maybe to one or two other targets) to the commit message.

Tue, Jan 12, 8:00 AM · Restricted Project
nemanjai committed rG3f7b4ce96065: [PowerPC] Add support for embedded devices with EFPU2 (authored by nemanjai).
[PowerPC] Add support for embedded devices with EFPU2
Tue, Jan 12, 7:50 AM
nemanjai closed D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2).
Tue, Jan 12, 7:50 AM · Restricted Project, Restricted Project
nemanjai accepted D94450: [LegalizeDAG][RISCV][PowerPC][AMDGPU][WebAssembly] Improve expansion of SETONE/SETUEQ on targets without SETO/SETUO..

LGTM for PPC. Thanks for this improvement.

Tue, Jan 12, 4:54 AM · Restricted Project

Mon, Jan 11

nemanjai added a comment to D94183: [PowerPC] Fix missing nop after call to weak callee. (llvm 10.x).

@abebeos Unfortunately there is no method for us to backport this to LLVM 10.x. Older releases are not maintained. There is typically a single point release for bug fixes after every major release and we have unfortunately missed the deadline for 11.0.1 with this so it will only be in 12.0.0.
Is it possible to maintain this patch as a necessary fix for LLVM 10 in Numba?

Mon, Jan 11, 12:12 PM · Restricted Project
nemanjai accepted D94385: [NFC] Disallow unused prefixes in CodeGen/PowerPC tests..

Thanks for doing this. I think it is useful for test cases to fail if a check prefix is unused (or becomes unused). LGTM.

Mon, Jan 11, 7:11 AM · Restricted Project

Fri, Jan 8

nemanjai added inline comments to D94058: [PowerPC] Sign extend comparison operand for signed atomic comparisons.
Fri, Jan 8, 7:50 AM · Restricted Project
nemanjai accepted D94162: [PowerPC] Add variants of 64-bit vector types for vec_sel..

Kind of bizarre that we missed these, thanks for adding them. LGTM.

Fri, Jan 8, 3:02 AM · Restricted Project
nemanjai added a comment to D94282: [PowerPC] Support ppc-asm-full-reg-names for AIX.

Historically, the letter prefixes were always stripped out because AIX and Linux assemblers don't accept them. The mentioned patch added the ability to produce prefixes with percent sign and letter which is actually accepted by the Linux assembler (GNU as). It was thought (not known) at the time that AIX does not accept this %<letter> syntax either so the guard for AIX was kept. If you can confirm that the AIX system assembler accepts this syntax, then this is fine.

Fri, Jan 8, 2:47 AM · Restricted Project

Tue, Jan 5

nemanjai accepted D94113: [PowerPC] Fix issue where vsrq is given incorrect shift vector.

LGTM.

Tue, Jan 5, 2:41 PM · Restricted Project
nemanjai added a comment to D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2).

Regarding similar GCC options:

AFAIK GCC has had the spe options -msingle-float and -mdouble-float until spe support was dropped after version 8.3.
These options would kind of match here, but they are already used as MIPS subtarget features in LLVM and I was not able to find out if and how it is possible to use identical target features for multiple targets and if this is a good idea at all? If somebody could point out how this can be done I'm happy to change the options...

Tue, Jan 5, 6:47 AM · Restricted Project, Restricted Project
nemanjai added a comment to D94054: [PowerPC] Try to fold sqrt/sdiv test results with the branch..

It seems like this patch depends on another patch. Please don't forget to add the dependencies to the Phabricator reviews.

Tue, Jan 5, 3:47 AM · Restricted Project

Mon, Jan 4

nemanjai requested review of D94058: [PowerPC] Sign extend comparison operand for signed atomic comparisons.
Mon, Jan 4, 7:57 PM · Restricted Project
nemanjai accepted D92935: Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2).

I don't really have any objections to this, but if there is a similar options to GCC, I would hope that they are the same. Please add a comment in the commit message to that end.
Other than that, LGTM.

Mon, Jan 4, 3:26 AM · Restricted Project, Restricted Project

Tue, Dec 29

nemanjai accepted D90156: [PowerPC] Do not fold `cmp(d|w)` and `subf` instruction to `subf.` if `nsw` is not present.

LGTM with a minor nit.

Tue, Dec 29, 5:06 AM · Restricted Project
nemanjai accepted D93092: [PowerPC] Remaining KnownBits should be constant when performing non-sign comparison.

LGTM. Thanks.

Tue, Dec 29, 4:54 AM · Restricted Project
nemanjai committed rG7486de1b2ece: [PowerPC] Provide patterns for permuted scalar to vector for pre-P8 (authored by nemanjai).
[PowerPC] Provide patterns for permuted scalar to vector for pre-P8
Tue, Dec 29, 4:50 AM
nemanjai added inline comments to D93092: [PowerPC] Remaining KnownBits should be constant when performing non-sign comparison.
Tue, Dec 29, 3:49 AM · Restricted Project
nemanjai committed rG0a19fc3088f5: [PowerPC] Disable CTR loops containing operations on half-precision (authored by nemanjai).
[PowerPC] Disable CTR loops containing operations on half-precision
Tue, Dec 29, 3:13 AM

Mon, Dec 28

nemanjai committed rG4f568fbd2163: [PowerPC] Do not emit HW loop when TLS var accessed in PHI of loop exit (authored by nemanjai).
[PowerPC] Do not emit HW loop when TLS var accessed in PHI of loop exit
Mon, Dec 28, 6:37 PM
nemanjai added inline comments to D92473: [Legalizer] Promote result type in expanding FP_TO_XINT.
Mon, Dec 28, 8:15 AM · Restricted Project
nemanjai requested changes to D92815: [PowerPC] [Clang] Enable float128 feature on Altivec targets.

I understand that the back end only needs Altivec to support the operations, but enabling this on non-vsx subtargets is different from what GCC does. I don't see a good reason for us to depart from that.

Mon, Dec 28, 7:56 AM
nemanjai added inline comments to D93707: [FPEnv] Allow fneg + strict_fsub -> strict_fadd in DAGCombiner.
Mon, Dec 28, 7:40 AM · Restricted Project
nemanjai added inline comments to D89195: [PowerPC][Power10] Exploit store rightmost vector element instructions..
Mon, Dec 28, 7:27 AM · Restricted Project, Restricted Project
nemanjai committed rGe73f885c988d: [PowerPC] Remove redundant COPY_TO_REGCLASS introduced by 8a58f21f5b6c (authored by nemanjai).
[PowerPC] Remove redundant COPY_TO_REGCLASS introduced by 8a58f21f5b6c
Mon, Dec 28, 7:27 AM
nemanjai added a comment to D91050: [NFC] Add the EmitTargetCodeForConstantPool hook for target to customize it with MachineConstantPoolValue.

Hmm... I think it is very surprising if DAG.getConstantPool() does not return ConstantPoolSDNode. I am not in favour of any design that violates such seemingly reasonable assumptions.

Mon, Dec 28, 4:28 AM · Restricted Project
nemanjai added a comment to D92083: [PowerPC] Lower the SETCC/SELECT_CC/BR_CC as libcall for fp128 with Power9 vector disabled.

Please explain in the description of the review (and subsequent commit message) why custom lowering is needed. I suppose for SELECT_CC, it is needed because using setOperationAction() uses the result type which is only guaranteed to match the true/false value but not the comparison types. However, it is not immediately obvious to me why we need custom lowering for BR_CC.

Mon, Dec 28, 4:10 AM · Restricted Project

Tue, Dec 22

nemanjai committed rGba1202a1e4f7: [PowerPC] Restore stack ptr from base ptr when available (authored by nemanjai).
[PowerPC] Restore stack ptr from base ptr when available
Tue, Dec 22, 3:44 AM
nemanjai closed D93327: [PowerPC] Restore stack ptr from base ptr when available.
Tue, Dec 22, 3:44 AM · Restricted Project, Restricted Project
nemanjai added a comment to D91053: [PowerPC] Lump the constants to save one addis for each constant access.

The test you are looking for is llvm/test/CodeGen/PowerPC/constant-pool.ll in fact.

Tue, Dec 22, 2:46 AM · Restricted Project
nemanjai added a comment to D91053: [PowerPC] Lump the constants to save one addis for each constant access.

There are two parent revisions that this patch depends on. You need to apply them first. I will add a new test to summarize all the pattern we can handle.

Which ones? I don't see them listed in "Parent Revisions"

Tue, Dec 22, 2:15 AM · Restricted Project

Mon, Dec 21

nemanjai added a comment to D93377: [Clang] Add __ibm128 type to represent ppc_fp128.

Seems that conversion diagnostic test cases are completely missing.

Mon, Dec 21, 4:48 PM · Restricted Project
nemanjai accepted D92054: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables.

LGTM.

Mon, Dec 21, 3:05 PM · Restricted Project
nemanjai added a comment to D90807: [PowerPC] add has side effect for SAT bit clobber intrinsics/instructions.

Thank you so much for fixing this and I'm sorry I didn't get around to going over it sooner.

Mon, Dec 21, 2:11 PM · Restricted Project
nemanjai added inline comments to D90131: [PowerPC] Add folding patterns for rlwinm + andi_rec..
Mon, Dec 21, 2:07 PM · Restricted Project
nemanjai accepted D90329: [PowerPC] Fix va_arg in C++, Objective-C on 32-bit ELF targets.

LGTM aside from a minor nit.

Mon, Dec 21, 12:15 PM · Restricted Project
nemanjai added a comment to D89855: [PowerPC] Extend folding RLWINM + RLWINM to post-RA..

I have added a number of comments to code that isn't part of this review which may seem odd. However, you are adding a fair amount of code to a function that is already very long/complex and rather than doing so, I'd prefer that you refactor the original function so the new code integrates more seamlessly.

Mon, Dec 21, 10:35 AM · Restricted Project
nemanjai requested changes to D91053: [PowerPC] Lump the constants to save one addis for each constant access.

Rebase the patch.

Mon, Dec 21, 8:33 AM · Restricted Project
nemanjai added a comment to D93370: [PowerPC] Add new infrastructure to select load/store instructions, update P8/P9 load/store patterns..

So for the new infra, I am thinking should we avoid this redundant logic? I still see redundant checks/flags collects for one load/store instruction. We first check SelectDSForm and then check SelectXForm and then SelectForceXForm according to the order in td files. In each SelectXXXForm(), we will collect the flags once. There should also be some redundant logics?

Can we select the address mode not starting from td files? Instead, we start the selection from cpp file for IR level load? For example in function PPCDAGToDAGISel::Select(), change case ISD::LOAD and inside the case, we call SelectOptimalAddrMode and then select the PPC instruction directly in cpp files.

Mon, Dec 21, 6:52 AM · Restricted Project, Restricted Project

Dec 17 2020

nemanjai added a comment to D93370: [PowerPC] Add new infrastructure to select load/store instructions, update P8/P9 load/store patterns..

Just my general $0.02 regarding this refactoring effort...

Dec 17 2020, 3:41 AM · Restricted Project, Restricted Project
nemanjai accepted D93336: [PowerPC][NFC] Cleanup PPCCTRLoopsVerify pass.
Dec 17 2020, 3:17 AM · Restricted Project
nemanjai added a comment to D93336: [PowerPC][NFC] Cleanup PPCCTRLoopsVerify pass.

LGTM.

Dec 17 2020, 3:16 AM · Restricted Project
nemanjai accepted D90349: [DAGCombiner] Improve shift by select of constant.

I am really sorry that I've missed these notifications. The PPC changes are perfectly fine. This might lead to regressions if the shift amount is large enough to require more than a single instruction to produce each constant (i.e. the constant no longer fits in a 16-bit signed immediate). But there's work underway to improve constant materialization in the back end that will alleviate this problem.

Dec 17 2020, 2:54 AM · Restricted Project

Dec 15 2020

nemanjai requested review of D93327: [PowerPC] Restore stack ptr from base ptr when available.
Dec 15 2020, 11:48 AM · Restricted Project, Restricted Project

Dec 14 2020

nemanjai committed rGeed0b9acdfe4: [PowerPC] Temporarily disable asan longjmp tests (authored by nemanjai).
[PowerPC] Temporarily disable asan longjmp tests
Dec 14 2020, 4:22 PM
nemanjai committed rGbfdc19e77868: [PowerPC] Restore stack ptr from frame ptr with setjmp (authored by nemanjai).
[PowerPC] Restore stack ptr from frame ptr with setjmp
Dec 14 2020, 9:35 AM
nemanjai closed D92906: [PowerPC] Restore stack ptr from frame ptr with setjmp.
Dec 14 2020, 9:35 AM · Restricted Project

Dec 11 2020

nemanjai requested changes to D90173: [PowerPC] Exploit splat instruction xxsplti32dx in Power10.

This is not functionally correct.

Dec 11 2020, 7:06 AM · Restricted Project, Restricted Project, Restricted Project
nemanjai accepted D93099: [PowerPC] Select the D-Form load if we know its offset meets the requirement.

LGTM.
Most of this will be going away soon since we're refactoring how we select memory access instructions, but that's fine.

Dec 11 2020, 6:49 AM · Restricted Project

Dec 8 2020

nemanjai requested review of D92906: [PowerPC] Restore stack ptr from frame ptr with setjmp.
Dec 8 2020, 7:42 PM · Restricted Project

Dec 2 2020

nemanjai added inline comments to D90131: [PowerPC] Add folding patterns for rlwinm + andi_rec..
Dec 2 2020, 3:43 AM · Restricted Project
nemanjai added inline comments to D90131: [PowerPC] Add folding patterns for rlwinm + andi_rec..
Dec 2 2020, 3:39 AM · Restricted Project

Dec 1 2020

nemanjai accepted D91391: [PowerPC] Fix for excessive ACC copies due to PHI nodes.

My comments are all cosmetic, feel free to address them on the commit. LGTM otherwise.

Dec 1 2020, 7:39 PM · Restricted Project
nemanjai added a comment to D92445: [PowerPC] Enable OpenMP for powerpcle target. [5/5].

This seems problematic to me for a few reasons:

  1. There is no 32-bit toolchains or libraries for little endian Linux systems
  2. There is no support in the ELFv2 ABI for 32-bit object mode and there may be a number of places we assume that little endian systems use ELFv2 ABI
  3. There are a number of places that make the assumption that little endian systems must be 64-bit so there's no checking for 32-bit mode (which will likely result in 64-bit specific code gen)
Dec 1 2020, 7:22 PM · Restricted Project, Restricted Project, Restricted Project
nemanjai accepted D92420: [PowerPC] Exploitation of xxeval instruction for AND and NAND.

I think Baptiste's suggestion to add the expression prior to each pattern is a good idea.

Dec 1 2020, 4:45 PM · Restricted Project
nemanjai added a comment to D92420: [PowerPC] Exploitation of xxeval instruction for AND and NAND.

LGTM as long as the test case is updated.

Dec 1 2020, 4:44 PM · Restricted Project

Nov 24 2020

nemanjai added a comment to D91611: [PowerPC][LLD] Detecting and fixing missing TLS relocation on __tls_get_addr.

I don't think adding an option to GNU ld is appropriate because it does the slow thing by default. Stefan is working on some test cases to illustrate GNU ld's behaviour so I think we should start with what it does and see whether we want to match behaviour and whether we want to do so under option control.

Nov 24 2020, 2:00 PM · Restricted Project
nemanjai added a comment to D91611: [PowerPC][LLD] Detecting and fixing missing TLS relocation on __tls_get_addr.

I am still questioning the motivation behind this change. If there are indeed ABI issues from older compilers,
can they add a wrapper around the linker if the check is deemed useful?

I don't really see it as an improvement in usability to have to post-process object files, potentially introducing another possible point of failure vs. providing an option to the linker.

(In addition, binutils does not have --add-missing-tls-reloc.)

I think that a departure from option compatibility is reasonable here if we can provide an option that will:

  • Allow us to handle the missing relocation when the option is specified
  • Not introduce any performance overhead when the option is not specified (i.e. the default behaviour)
Nov 24 2020, 7:19 AM · Restricted Project
nemanjai accepted D91527: [PowerPC][FP128] Fix the incorrect calling convention for IEEE long double on Power8.

LGTM.

Nov 24 2020, 3:38 AM · Restricted Project
nemanjai accepted D89938: [DAG][PowerPC] Fix dropped `nsw` flag in `SimplifySetCC` by adding `doesNodeExist` helper.

LGTM. This seems like a reasonable solution.

Nov 24 2020, 3:24 AM · Restricted Project

Nov 20 2020

nemanjai added inline comments to D91603: [PowerPC] Correct the bit-width definition for some imm operand in td..
Nov 20 2020, 4:55 AM · Restricted Project
nemanjai added a comment to D91603: [PowerPC] Correct the bit-width definition for some imm operand in td..

(Not relevant question) Do we have intrinsics, or any exploitation in library for darn?

Nov 20 2020, 4:40 AM · Restricted Project
nemanjai accepted D91846: [SelectionDAG][X86][PowerPC][Mips] Replace the default implementation of LowerOperationWrapper with the X86 and PowerPC version..

LGTM. Thanks for commoning this.

Nov 20 2020, 4:33 AM · Restricted Project
nemanjai added a comment to D91331: Expand the fp_to_int/int_to_fp/fp_round/fp_extend as libcall for fp128.

It would appear to me that the only test case changes are that the libcalls were turned into tail calls - but of course my knowledge of X86 and ARM asm is next to zero. Presumably the original lowering didn't set the necessary flags on the call and now the legalizer does so.

Nov 20 2020, 4:00 AM · Restricted Project
nemanjai accepted D91265: [PowerPC] Don't reuse the address of an illegal typed load for int_to_fp..

The patch LGTM, but I really think the test case should change to more clearly convey the behaviour you desire to test.

Nov 20 2020, 3:45 AM · Restricted Project, Restricted Project

Nov 19 2020

nemanjai added a comment to D91331: Expand the fp_to_int/int_to_fp/fp_round/fp_extend as libcall for fp128.

I know that Simon mentioned that perhaps we should limit this to cast operations but TBH, I think there may be value in making this general and passing in the SDNode itself rather than opcode/type(s). Then all of this logic can be greatly simplified into something like this:

void SelectionDAGLegalize::LegalizeOp(SDNode *Node) {
  LLVM_DEBUG(dbgs() << "\nLegalizing: "; Node->dump(&DAG));
  if (TLI.opRequiresLibCall(Node))
    ConvertNodeToLibcall(Node);

And presumably, there would be another function for the TLI to provide the RTLIB::Libcall for the specific SDNode.

Nov 19 2020, 6:38 AM · Restricted Project

Nov 5 2020

nemanjai added a comment to D90807: [PowerPC] add has side effect for SAT bit clobber intrinsics/instructions.

This change is fine as it is. However, since we don't model the VSCR and we mark saturating intrinsics as having no sideeffects, we can miscompile wrt. this instruction/intrinsic.
Here is an example where we schedule a saturating instruction before the mfvsrc where the user clearly wanted it after:

vector int test(vector int a, vector int b, vector int aa,
                vector signed short *__restrict FromVSCR) {
  vector int c = __builtin_altivec_vsumsws(a, b);
Nov 5 2020, 5:11 AM · Restricted Project

Nov 3 2020

nemanjai accepted D84962: [PowerPC] Correct cpsgn's behaviour on PowerPC to match that of the ABI.

LGTM.

Nov 3 2020, 8:58 AM · Restricted Project, Restricted Project, Restricted Project

Oct 27 2020

nemanjai committed rG5459d08795e3: [PowerPC] Fix single-use check and update chain users for ld-splat (authored by nemanjai).
[PowerPC] Fix single-use check and update chain users for ld-splat
Oct 27 2020, 2:50 PM

Oct 23 2020

nemanjai added inline comments to D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility.
Oct 23 2020, 2:42 AM · Restricted Project

Oct 15 2020

nemanjai accepted D88746: [zorg] [PowerPC] Limit number of threads to 20 on clang-ppc64le-rhel buildbot.

Forgot to accept.

Oct 15 2020, 11:04 AM · Restricted Project
nemanjai added a comment to D88746: [zorg] [PowerPC] Limit number of threads to 20 on clang-ppc64le-rhel buildbot.

LGTM. Let's reduce this until we can add the option to limit the number of threads for LLD into lit configs.

Oct 15 2020, 11:03 AM · Restricted Project

Oct 8 2020

nemanjai added inline comments to D88154: Initial support for vectorization using Libmvec (GLIBC vector math library)..
Oct 8 2020, 3:54 AM · Restricted Project, Restricted Project

Oct 7 2020

nemanjai accepted D75059: [PowerPC] Remove support for VRSAVE save/restore/update..

LGTM. Nice to have this cleaned up. Thanks.

Oct 7 2020, 5:43 AM · Restricted Project, Restricted Project
nemanjai added inline comments to D84962: [PowerPC] Correct cpsgn's behaviour on PowerPC to match that of the ABI.
Oct 7 2020, 5:29 AM · Restricted Project, Restricted Project, Restricted Project
nemanjai accepted D88716: [TwoAddressInstruction][PowerPC] Call `regOverlapsSet` to find out real clobbers and uses.

LGTM. Checking for overlap rather than simply the same register should be the right thing to do. However, I wonder if we might have other places where a similar assumption is made that could become a problem since we presumably only mark the subregister as an implicit def.

Oct 7 2020, 5:24 AM · Restricted Project

Oct 5 2020

nemanjai added a comment to D82485: Add tests for sequences of callbacks that RecursiveASTVisitor produces.

Hi @gribozavr do you think we can do something about this test?

Oct 5 2020, 11:08 AM · Restricted Project

Sep 29 2020

nemanjai accepted D88274: [PowerPC] Put the CR field in low bits of GRC during copying CRRC to GRC..

LGTM. The code here is correct, but I do have a couple of questions:

  1. Do these copies come up in real code? And if so, under which circumstances. We don't have copies in the other direction implemented, so that makes me wonder when these ever come up.
  2. Does this solve a problem seen in real code? If so, is there a bugzilla report? At the very least, the conditions under which this causes a problem should be added to the description.
  3. Is there any follow up required/planned for this?
Sep 29 2020, 5:17 AM · Restricted Project

Sep 24 2020

nemanjai accepted D87046: [PPC] Do not emit extswsli in 32BIT mode when using -mcpu=pwr9.

This LGTM. My comment isn't really meant to require you to change the test, just to indicate that there are a lot of moving parts that are very difficult to account for completely. So if update_llc_test_checks.py works, you're better off just using that.

Sep 24 2020, 7:50 AM · Restricted Project
nemanjai accepted D86819: [PowerPC][Power10] Implementation of 128-bit Binary Vector Rotate builtins.

The remaining requests can be fulfilled when committing. I don't think this requires another review. Thanks.

Sep 24 2020, 6:46 AM · Restricted Project, Restricted Project, Restricted Project
nemanjai requested changes to D84962: [PowerPC] Correct cpsgn's behaviour on PowerPC to match that of the ABI.

This is not what we want. The builtin behaves correctly. It is equivalent to the generic __builtin_copysign and it would be very surprising to a user if it reverses the operands depending on which version you use. What is implemented incorrectly is vec_cpsgn so that's what should change.

Sep 24 2020, 4:34 AM · Restricted Project, Restricted Project, Restricted Project
nemanjai added a comment to D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex.

This clearly changes behaviour and should thereby not have the [NFC] tag.

Sep 24 2020, 4:15 AM · Restricted Project
nemanjai added a comment to D88044: [llvm-exegesis][PowerPC] Add more register classes.

I think the code being added here (and perhaps the existing code as well) requires deep expertise in both PowerPC ISA and Exegesis. But I think with more descriptive comments, we can make it consumable for those that only check one of those boxes. So most of my comments are requests to add more descriptive/explanatory comments.

Sep 24 2020, 4:09 AM · Restricted Project

Sep 23 2020

nemanjai accepted D83500: [PowerPC][Power10] Implement custom codegen for the vec_replace_elt and vec_replace_unaligned builtins..

LGTM. Thanks for your patience and for addressing all the comments.

Sep 23 2020, 11:14 AM · Restricted Project, Restricted Project, Restricted Project
nemanjai added inline comments to D83500: [PowerPC][Power10] Implement custom codegen for the vec_replace_elt and vec_replace_unaligned builtins..
Sep 23 2020, 9:10 AM · Restricted Project, Restricted Project, Restricted Project

Sep 22 2020

nemanjai added a comment to rG144e57fc9535: [Sanitizers] Implement interceptors for msgsnd, msgrcv.

The commit that mentions this (https://reviews.llvm.org/rGf1746be6667) adds the cleanup. Please have a look at it and if it isn't the right way to clean up, modify the fix as needed. I had to make sure this doesn't continue to build up queues as it was causing build bot failures.

Sep 22 2020, 9:24 PM
nemanjai committed rGf1746be66673: [Sanitizers] Fix test case that doesn't clean up after itself (authored by nemanjai).
[Sanitizers] Fix test case that doesn't clean up after itself
Sep 22 2020, 9:22 PM
nemanjai added a comment to D87921: Fix -funique-internal-linkage-names to work with -O2 and new pass manager.

It turns out that the culprit for the PPC bot failures is actually https://reviews.llvm.org/rG144e57fc9535
But this just took a while to manifest.

Sep 22 2020, 8:42 PM · Restricted Project
nemanjai added a comment to rG144e57fc9535: [Sanitizers] Implement interceptors for msgsnd, msgrcv.

The test case added in this patch creates System V message queues that it never cleans up. Why is that? We now have this failing on a machine that runs a lot of our bots. Presumably it will start failing on any machine that runs bots after enough runs.

Sep 22 2020, 8:41 PM
nemanjai added a comment to D87921: Fix -funique-internal-linkage-names to work with -O2 and new pass manager.

The revert did not fix the PPC bots. I suspect there is some kind of resource issue from the logs:

msgget:: No space left on device
sysmsg.c.tmp: /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/llvm/compiler-rt/test/sanitizer_common/TestCases/Linux/sysmsg.c:15: int main(): Assertion `msgq != -1' failed.

http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt
http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux

Sep 22 2020, 8:27 PM · Restricted Project

Sep 21 2020

nemanjai accepted D87394: [PowerPC][Power10] Implementation of 128-bit Binary Vector Mod and Sign Extend builtins.

The nits can be addressed when committing the code. LGTM otherwise.

Sep 21 2020, 3:54 AM · Restricted Project, Restricted Project, Restricted Project