Page MenuHomePhabricator

sfertile (Sean Fertile)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 24 2016, 8:15 AM (180 w, 1 d)

Recent Activity

Today

sfertile added a comment to D77578: [AIX][PPC] Implement caller byval arguments in stack memory.

Can you make this dependent on https://reviews.llvm.org/D76902and rebase/repost.

Tue, Apr 7, 8:07 AM · Restricted Project

Yesterday

sfertile updated the diff for D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.
  • rebase to pick up landed dependencies.
  • Moved assertion that loop index was still valid to top of loop.
  • added a test that exercise fatal error for args split between registers and stack.
Mon, Apr 6, 2:43 PM · Restricted Project, Restricted Project
sfertile accepted D77564: [PowerPC] Do not attempt to reuse load for 64-bit FP_TO_UINT without FPCVT.

Which I think mishandles negative values.

Mon, Apr 6, 2:10 PM · Restricted Project
sfertile added a comment to D77564: [PowerPC] Do not attempt to reuse load for 64-bit FP_TO_UINT without FPCVT.

run clang-format, but otherwise for fixing the assertion it LGTM. FWIW I'm not sure the code we produce for the conversion handles negative numbers correctly. For the reduced IR I posted I get:

Mon, Apr 6, 1:37 PM · Restricted Project
sfertile accepted D77235: [NFC][PowerPC] Cleanup 64-bit and Darwin CalleeSavedRegs.

Thanks for the update, LGTM.

Mon, Apr 6, 9:11 AM · Restricted Project
sfertile added a reviewer for D77235: [NFC][PowerPC] Cleanup 64-bit and Darwin CalleeSavedRegs: Restricted Project.
Mon, Apr 6, 9:11 AM · Restricted Project

Fri, Apr 3

sfertile added a comment to D77235: [NFC][PowerPC] Cleanup 64-bit and Darwin CalleeSavedRegs.

I would rather we don't rename any of the ViaCopy lists, or CSR_SRV464_TLS_PE_SaveList since they are likely dead. We can write IR that triggers them, but Clang will only generate said IR for Darwin IIUC and from the name of the calling convention it is limited to C++.
Also please clang format the changes.

Fri, Apr 3, 8:02 AM · Restricted Project

Thu, Apr 2

sfertile accepted D76380: [PowerPC][AIX] Implement by-val caller arguments in multiple registers.

LGTM.

Thu, Apr 2, 2:06 PM · Restricted Project
sfertile added inline comments to D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.
Thu, Apr 2, 8:06 AM · Restricted Project, Restricted Project

Wed, Apr 1

sfertile added a child revision for D76380: [PowerPC][AIX] Implement by-val caller arguments in multiple registers: D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.
Wed, Apr 1, 5:27 PM · Restricted Project
sfertile added parent revisions for D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers: D76875: [NFC] [PPC] [AIX] Test improvements for byval arguments that fit in a single register, D76380: [PowerPC][AIX] Implement by-val caller arguments in multiple registers.
Wed, Apr 1, 5:27 PM · Restricted Project, Restricted Project
sfertile added a child revision for D76875: [NFC] [PPC] [AIX] Test improvements for byval arguments that fit in a single register: D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.
Wed, Apr 1, 5:27 PM · Restricted Project
sfertile updated the diff for D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.

Rebased

Wed, Apr 1, 3:45 PM · Restricted Project, Restricted Project
sfertile added inline comments to D77235: [NFC][PowerPC] Cleanup 64-bit and Darwin CalleeSavedRegs.
Wed, Apr 1, 1:30 PM · Restricted Project
sfertile added a comment to D76875: [NFC] [PPC] [AIX] Test improvements for byval arguments that fit in a single register.

LGTM. Thanks for the updates Chris.

Wed, Apr 1, 12:57 PM · Restricted Project
sfertile added a comment to D76380: [PowerPC][AIX] Implement by-val caller arguments in multiple registers.

I suggest adding a test along the lines of:

Wed, Apr 1, 11:49 AM · Restricted Project
sfertile added a comment to D77101: [AIX] Return the correct set of callee saved regs.

Change looks good, however there is a couple things complicating this.

  1. The SplitCSR code is dead with Darwins removal and we didn't realize it.
  2. CSR_AIX64 and CSR_SVR464 are the same and should be one list, not 2 distinct lists.
Wed, Apr 1, 7:52 AM · Restricted Project

Tue, Mar 31

sfertile accepted D76907: [PPCInstPrinter] Print conditional branches as `bt 2, $target` instead of `bt 2, .+$imm`.

1 comment, but otherwise LGTM.

Tue, Mar 31, 2:53 PM · Restricted Project
sfertile added inline comments to D76380: [PowerPC][AIX] Implement by-val caller arguments in multiple registers.
Tue, Mar 31, 12:33 PM · Restricted Project
sfertile updated the diff for D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.

Add a test that checks for a fatal error when a ByVal argument is split across registers and stack.

Tue, Mar 31, 12:33 PM · Restricted Project, Restricted Project
sfertile added a comment to D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.

A small suggestion would be to add a test to further show that the GPRs are "burned" properly with with floats in the byval handling. Eg. using a struct like `%struct.S31 = type <{ float, i32, i64, double, i32, double }>' and seeing the error that structs split regs and the stack are not handled.

This is a really good suggestion. Going to add this next.

Tue, Mar 31, 7:09 AM · Restricted Project, Restricted Project

Mon, Mar 30

sfertile added a comment to D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.

A small suggestion would be to add a test to further show that the GPRs are "burned" properly with with floats in the byval handling. Eg. using a struct like `%struct.S31 = type <{ float, i32, i64, double, i32, double }>' and seeing the error that structs split regs and the stack are not handled.

Mon, Mar 30, 9:10 AM · Restricted Project, Restricted Project
sfertile updated the diff for D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.
  • Fixed wording on a fatal error.
  • Changed the args and captures on the lambda.
  • Restructured the loop as suggested.
  • Added comment about the dead-stores in the lit test.
Mon, Mar 30, 9:10 AM · Restricted Project, Restricted Project
sfertile added inline comments to D76875: [NFC] [PPC] [AIX] Test improvements for byval arguments that fit in a single register.
Mon, Mar 30, 8:03 AM · Restricted Project

Fri, Mar 27

sfertile accepted D76875: [NFC] [PPC] [AIX] Test improvements for byval arguments that fit in a single register.

One comment, but otherwise LGTM.

Fri, Mar 27, 10:16 AM · Restricted Project
sfertile accepted D76773: [PowerPC] Don't generate ST_VSR_SCAL_INT if power8-vector is disabled, fix PR45297.

Thank you for the updates. One minor comment, but patch LGTM.

Fri, Mar 27, 8:30 AM · Restricted Project

Thu, Mar 26

sfertile created D76902: [PowerPC][AIX] ByVal formal argument support: multiple registers.
Thu, Mar 26, 9:12 PM · Restricted Project, Restricted Project
sfertile added a comment to D76773: [PowerPC] Don't generate ST_VSR_SCAL_INT if power8-vector is disabled, fix PR45297.

A couple minor suggestions. If you agree with them, feel free to land them as an NFC update to the test before commiting this patch.

Thu, Mar 26, 2:08 PM · Restricted Project

Wed, Mar 25

sfertile committed rG3282d875d6f7: [PowerPC][AIX] ByVal formal arguments in a single register. (authored by sfertile).
[PowerPC][AIX] ByVal formal arguments in a single register.
Wed, Mar 25, 8:37 AM
sfertile closed D76401: [PowerPC][AIX] ByVal formal argument support: single register..
Wed, Mar 25, 8:37 AM · Restricted Project, Restricted Project
sfertile accepted D76591: [PPCInstPrinter] Change printBranchOperand(calltarget) to print the target address in hexadecimal form.

LGTM.

Wed, Mar 25, 6:59 AM · Restricted Project

Tue, Mar 24

sfertile added inline comments to D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang.
Tue, Mar 24, 3:39 PM · Restricted Project
sfertile added a comment to D76591: [PPCInstPrinter] Change printBranchOperand(calltarget) to print the target address in hexadecimal form.

@nemanjai
AIX system assembler does not seem to like this new form.

Assembler:
test.s: line 46: 1252-086 The target of the branch instruction
        must be a relocatable or external expression.

llvm-objdump -d output may not be assembled back. That said, I think most users like the PC relative immediate to be displayed as the target address. If AIX users don't agree, I can special case it.

Tue, Mar 24, 1:28 PM · Restricted Project
sfertile updated the diff for D76401: [PowerPC][AIX] ByVal formal argument support: single register..

Added a comment to describe why we must store the registers used to pas a ByVal to the stack.

Tue, Mar 24, 1:28 PM · Restricted Project, Restricted Project
sfertile added inline comments to D76401: [PowerPC][AIX] ByVal formal argument support: single register..
Tue, Mar 24, 12:21 PM · Restricted Project, Restricted Project
sfertile updated the diff for D76401: [PowerPC][AIX] ByVal formal argument support: single register..
  • split the ByVal handling into memory and register parts to remove an 'else'.
  • add back some missing info for fixed stack objects in the lit test.
  • remove check lines related to fixed stack objects which are not semantically meaningful.
Tue, Mar 24, 11:49 AM · Restricted Project, Restricted Project
sfertile added inline comments to D76401: [PowerPC][AIX] ByVal formal argument support: single register..
Tue, Mar 24, 11:49 AM · Restricted Project, Restricted Project
sfertile added inline comments to D76591: [PPCInstPrinter] Change printBranchOperand(calltarget) to print the target address in hexadecimal form.
Tue, Mar 24, 10:43 AM · Restricted Project

Mon, Mar 23

sfertile added inline comments to D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang.
Mon, Mar 23, 10:21 AM · Restricted Project
sfertile updated the diff for D76401: [PowerPC][AIX] ByVal formal argument support: single register..

Run clang-format.

Mon, Mar 23, 10:21 AM · Restricted Project, Restricted Project
sfertile added inline comments to D76401: [PowerPC][AIX] ByVal formal argument support: single register..
Mon, Mar 23, 9:14 AM · Restricted Project, Restricted Project
sfertile updated the diff for D76401: [PowerPC][AIX] ByVal formal argument support: single register..
  • Fixed assertion with zero sized byvals that new testing exposed.
  • Addressed various review comments (renamed TrueSize, use alignTo, etc).
Mon, Mar 23, 8:41 AM · Restricted Project, Restricted Project

Fri, Mar 20

sfertile committed rG56122fcd641a: [PowerPC][AIX][NFC] Extend the test coverage of ByVal args. (authored by sfertile).
[PowerPC][AIX][NFC] Extend the test coverage of ByVal args.
Fri, Mar 20, 9:45 AM
sfertile committed rGfc902cb6e2b2: [PowerPC][AIX][NFC] Add zero-sized by val params to cc test. (authored by sfertile).
[PowerPC][AIX][NFC] Add zero-sized by val params to cc test.
Fri, Mar 20, 8:39 AM
sfertile added a comment to D76401: [PowerPC][AIX] ByVal formal argument support: single register..

I'm going to implement the suggested changes to the testing (ie a zero-sized-byval and different byval types so we are loading more then just a char). I'll do so in a separate patch which makes the changes and shows the difference in the existing tests and then I'll rebase this patch onto that.

Fri, Mar 20, 7:00 AM · Restricted Project, Restricted Project
sfertile updated the diff for D76401: [PowerPC][AIX] ByVal formal argument support: single register..
  • rename local to match other uses in function.
  • constify a handful of locals.
Fri, Mar 20, 7:00 AM · Restricted Project, Restricted Project

Thu, Mar 19

sfertile added inline comments to D76380: [PowerPC][AIX] Implement by-val caller arguments in multiple registers.
Thu, Mar 19, 9:45 AM · Restricted Project
sfertile updated the diff for D76401: [PowerPC][AIX] ByVal formal argument support: single register..

Landed check-prefix changes as an NFC patch and rebased ontop of that.

Thu, Mar 19, 8:37 AM · Restricted Project, Restricted Project
sfertile committed rG06c810b1559e: [PowerPC][AIX] Simplify the check prefixes in the ByVal lit tests. [NFC] (authored by sfertile).
[PowerPC][AIX] Simplify the check prefixes in the ByVal lit tests. [NFC]
Thu, Mar 19, 8:06 AM

Wed, Mar 18

sfertile added inline comments to D76401: [PowerPC][AIX] ByVal formal argument support: single register..
Wed, Mar 18, 6:28 PM · Restricted Project, Restricted Project
sfertile created D76401: [PowerPC][AIX] ByVal formal argument support: single register..
Wed, Mar 18, 6:28 PM · Restricted Project, Restricted Project
sfertile committed rGc21866476e14: [PowerPC][AIX] Implement by-val caller arguments in a single register. (authored by cebowleratibm).
[PowerPC][AIX] Implement by-val caller arguments in a single register.
Wed, Mar 18, 8:09 AM
sfertile closed D75863: [AIX] Implement by-val caller arguments in a single register.
Wed, Mar 18, 8:09 AM · Restricted Project
sfertile added inline comments to D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang.
Wed, Mar 18, 8:08 AM · Restricted Project
sfertile accepted D75863: [AIX] Implement by-val caller arguments in a single register.

LGTM.

Wed, Mar 18, 6:30 AM · Restricted Project

Tue, Mar 17

sfertile added inline comments to D75863: [AIX] Implement by-val caller arguments in a single register.
Tue, Mar 17, 4:13 PM · Restricted Project

Mon, Mar 16

sfertile added a comment to D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX.

As a first step, I suggest we break the clang changes and the LLVM changes into 2 separate patches.

Mon, Mar 16, 10:52 AM · Restricted Project, Restricted Project

Thu, Mar 12

sfertile added inline comments to D75863: [AIX] Implement by-val caller arguments in a single register.
Thu, Mar 12, 10:51 AM · Restricted Project
sfertile committed rGd68831266033: [PowerPC][AIX] Implement formal arguments passed in stack memory. (authored by ZarkoCA).
[PowerPC][AIX] Implement formal arguments passed in stack memory.
Thu, Mar 12, 9:14 AM
sfertile closed D74225: [AIX] Implement formal arguments passed in stack memory.
Thu, Mar 12, 9:13 AM · Restricted Project
sfertile committed rG8b39341fb095: [PowerPC][AIX] Fix printing of program counter for AIX assembly. (authored by sfertile).
[PowerPC][AIX] Fix printing of program counter for AIX assembly.
Thu, Mar 12, 8:09 AM
sfertile closed D75627: [PowerPC][AIX] Fix printing of program counter for AIX assembly..
Thu, Mar 12, 8:08 AM · Restricted Project, Restricted Project

Wed, Mar 11

sfertile added inline comments to D75863: [AIX] Implement by-val caller arguments in a single register.
Wed, Mar 11, 9:04 AM · Restricted Project

Mon, Mar 9

sfertile accepted D74225: [AIX] Implement formal arguments passed in stack memory.

LGTM.

Mon, Mar 9, 7:30 AM · Restricted Project

Mar 6 2020

sfertile accepted D75702: [PowerPC32] Fix the `setcc` inconsistent result type problem.

1 minor comment but otherwise LGTM.

Mar 6 2020, 11:35 AM · Restricted Project

Mar 5 2020

sfertile added a comment to D75702: [PowerPC32] Fix the `setcc` inconsistent result type problem.

I get why you named the test ppc32-setcc-expansion.ll but to a casual reader its definitely unclear what a setcc has to do with this code. I would suggest 'ppc32-i64-to-float-conv.ll', and have a comment explaining how the lowering for the conversion generates a setcc which caused a crash under certain conditions.

Mar 5 2020, 1:12 PM · Restricted Project
sfertile accepted D75494: [PowerPC] Delete PPCMachObjectWriter and triple for darwin.

LGTM.

Mar 5 2020, 10:22 AM · Restricted Project, Restricted Project
sfertile committed rGc7b6fa8f4b86: [AIX] Extend int arguments to register width when passed in stack memory. (authored by cebowleratibm).
[AIX] Extend int arguments to register width when passed in stack memory.
Mar 5 2020, 9:18 AM
sfertile closed D75126: [AIX] Extend integer arguments to register width when passed in stack memory..
Mar 5 2020, 9:18 AM · Restricted Project
sfertile accepted D75126: [AIX] Extend integer arguments to register width when passed in stack memory..

LGTM.

Mar 5 2020, 7:08 AM · Restricted Project

Mar 4 2020

sfertile added inline comments to D75627: [PowerPC][AIX] Fix printing of program counter for AIX assembly..
Mar 4 2020, 1:29 PM · Restricted Project, Restricted Project
sfertile updated the diff for D75627: [PowerPC][AIX] Fix printing of program counter for AIX assembly..

Reworded comment and added 64-bit RUN step to the test.

Mar 4 2020, 1:29 PM · Restricted Project, Restricted Project
sfertile added inline comments to D74166: [AIX][Frontend] Static init implementation for AIX considering no priority.
Mar 4 2020, 12:56 PM · Restricted Project, Restricted Project
sfertile added a comment to D74166: [AIX][Frontend] Static init implementation for AIX considering no priority.

Please fix the formatting issues flagged by the pre-merge checks.

Mar 4 2020, 12:21 PM · Restricted Project, Restricted Project
sfertile created D75627: [PowerPC][AIX] Fix printing of program counter for AIX assembly..
Mar 4 2020, 10:39 AM · Restricted Project, Restricted Project
sfertile added inline comments to D75126: [AIX] Extend integer arguments to register width when passed in stack memory..
Mar 4 2020, 7:19 AM · Restricted Project

Mar 3 2020

sfertile committed rG383e3ec1b2ad: [PowerPC][NFC] Add missing expected output for AIX int stack arg test. (authored by cebowleratibm).
[PowerPC][NFC] Add missing expected output for AIX int stack arg test.
Mar 3 2020, 8:36 AM
sfertile committed rG65dd63fb33f0: [PowerPC][NFC] Lexically order expected output for AIX stack arg test. (authored by cebowleratibm).
[PowerPC][NFC] Lexically order expected output for AIX stack arg test.
Mar 3 2020, 8:35 AM

Feb 26 2020

sfertile added a comment to D74225: [AIX] Implement formal arguments passed in stack memory.

I think we need some changes to ensure frame objects are created with their proper size and not word size.

Feb 26 2020, 1:33 PM · Restricted Project
sfertile committed rG73c3b52676a1: [PowerPC][NFC] Convert grep usage to FileCheck in lit test. (authored by sfertile).
[PowerPC][NFC] Convert grep usage to FileCheck in lit test.
Feb 26 2020, 9:29 AM

Feb 25 2020

sfertile committed rGeb1c040b413a: [PowerPC][NFC] Remove comments mentioning Darwin and VRSAVE from lit test. (authored by sfertile).
[PowerPC][NFC] Remove comments mentioning Darwin and VRSAVE from lit test.
Feb 25 2020, 10:47 AM

Feb 24 2020

sfertile added a reviewer for D75059: [PowerPC][NFC] Remove support for VRSAVE save/restore/update.: jhibbits.
Feb 24 2020, 10:39 AM · Restricted Project, Restricted Project
sfertile created D75059: [PowerPC][NFC] Remove support for VRSAVE save/restore/update..
Feb 24 2020, 8:49 AM · Restricted Project, Restricted Project
sfertile committed rG8efc2f5723b0: [PowerPC][AIX] Spill/restore the callee-saved condition register bits. (authored by sfertile).
[PowerPC][AIX] Spill/restore the callee-saved condition register bits.
Feb 24 2020, 8:31 AM
sfertile closed D74349: [PowerPC][AIX] Spill and restore the non-volatile condition register bits..
Feb 24 2020, 8:31 AM · Restricted Project
sfertile added inline comments to D74349: [PowerPC][AIX] Spill and restore the non-volatile condition register bits..
Feb 24 2020, 7:35 AM · Restricted Project

Feb 21 2020

sfertile accepted D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate.

LGTM.

Feb 21 2020, 7:37 AM · Restricted Project, Restricted Project
sfertile committed rG175f6e309ab9: [PowerPC][NFC] Add a test for vrsave usage iinline asm. (authored by sfertile).
[PowerPC][NFC] Add a test for vrsave usage iinline asm.
Feb 21 2020, 7:00 AM
sfertile committed rG4fdaac0e1eb8: [PowerPC][NFC] Remove Darwin specific logic in frame finalization. (authored by sfertile).
[PowerPC][NFC] Remove Darwin specific logic in frame finalization.
Feb 21 2020, 6:33 AM

Feb 20 2020

sfertile committed rGda181d4ba0cd: [PowerPC][NFC] Cleanup some of the Darwin mentions in the README.txt. (authored by sfertile).
[PowerPC][NFC] Cleanup some of the Darwin mentions in the README.txt.
Feb 20 2020, 11:06 AM
sfertile accepted D74911: [AIX] Pack BasicBlockBits.

Fix the formatting that Lint called out, but otherwise LGTM for fixing building natively on AIX.

Feb 20 2020, 10:56 AM · Restricted Project
sfertile updated the diff for D74349: [PowerPC][AIX] Spill and restore the non-volatile condition register bits..

Fix formatting of CHECK lines in several of the lit tests.

Feb 20 2020, 10:20 AM · Restricted Project
sfertile committed rG45f008704df2: [PowerPC][NFC] We do not save/restore vrsave for any remaining subtargets. (authored by sfertile).
[PowerPC][NFC] We do not save/restore vrsave for any remaining subtargets.
Feb 20 2020, 7:26 AM

Feb 19 2020

sfertile added a comment to D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate.

Patch LGTM as far a formatting/naming/testing etc. C++ specifics is outside my wheelhouse though, so I can't confirm things like the tail padding rules are correct for AIX. Because of that I'm not comfortable being the one to accept the patch.

Feb 19 2020, 9:22 AM · Restricted Project, Restricted Project
sfertile updated the diff for D74349: [PowerPC][AIX] Spill and restore the non-volatile condition register bits..
  • Added a lambda for building the instruction which moves the condition register into a scratch register to avoid dupication.
  • Added an IR --> assembly test that verifies we restore all 3 non-volatile cr-fields.
  • Removed a couple stale comments, and reworded another comment.
Feb 19 2020, 8:05 AM · Restricted Project
sfertile added inline comments to D74349: [PowerPC][AIX] Spill and restore the non-volatile condition register bits..
Feb 19 2020, 8:05 AM · Restricted Project

Feb 18 2020

sfertile accepted D74631: [clang][XCOFF] Indicate that XCOFF does not support COMDATs.

LGTM.

Feb 18 2020, 12:26 PM · Restricted Project, Restricted Project
sfertile added a comment to rG63bb9fee525f: [llvm-exegesis] Improve error reporting in Assembler.cpp.

Looks like there are calls to assembleToStream which didn't get updated to check that the return value is valid, breaking bots that treat warnings as errors: http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/1389

Feb 18 2020, 9:12 AM
sfertile committed rG3126b556d13f: [PowerPC][NFC] Add defines to help creating the SpillSlot arrays. (authored by sfertile).
[PowerPC][NFC] Add defines to help creating the SpillSlot arrays.
Feb 18 2020, 8:53 AM

Feb 14 2020

sfertile added a comment to D74631: [clang][XCOFF] Indicate that XCOFF does not support COMDATs.
  1. We should probably update the COMDAT section of the lang ref to mention XCOFF doens't support COMDATS.
  2. Will we report an error somewhere in the backend if we encounter a COMDAT?
Feb 14 2020, 12:30 PM · Restricted Project, Restricted Project