Page MenuHomePhabricator

hubert.reinterpretcast (Hubert Tong)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 27 2014, 8:34 PM (281 w, 4 d)

Recent Activity

Fri, Jan 17

hubert.reinterpretcast added inline comments to D72347: [NFC][XCOFF] Refactor Csect creation into TargetLoweringObjectFile.
Fri, Jan 17, 2:43 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72347: [NFC][XCOFF] Refactor Csect creation into TargetLoweringObjectFile.
Fri, Jan 17, 8:34 AM · Restricted Project
hubert.reinterpretcast added a comment to D65761: Add Windows Control Flow Guard checks (/guard:cf)..

The CFGuard library shouldn't be needed for targets other than ARM, AArch64, and X86, and it's only being built for these targets. However, it looks like llvm-build is also including it in the LibraryDependencies.inc file for other targets, such as PowerPC, which I think is causing this error. I'm not sure how to indicate that the library is only needed for a specified subset of targets. @rnk or @hans any suggestions?

Fri, Jan 17, 7:55 AM · Restricted Project, Restricted Project

Thu, Jan 16

hubert.reinterpretcast added inline comments to D72479: [PowerPC][AIX] Make PIC the default relocation model for AIX.
Thu, Jan 16, 11:20 AM · Restricted Project
hubert.reinterpretcast added inline comments to D72347: [NFC][XCOFF] Refactor Csect creation into TargetLoweringObjectFile.
Thu, Jan 16, 10:32 AM · Restricted Project
hubert.reinterpretcast committed rG1794158f90f9: [MC][test] Fix non-portable GNU diff option (authored by hubert.reinterpretcast).
[MC][test] Fix non-portable GNU diff option
Thu, Jan 16, 8:38 AM
hubert.reinterpretcast closed D72745: [MC][test] Fix non-portable GNU diff option.
Thu, Jan 16, 8:38 AM · Restricted Project
hubert.reinterpretcast accepted D72479: [PowerPC][AIX] Make PIC the default relocation model for AIX.

LGTM.

Thu, Jan 16, 8:19 AM · Restricted Project
hubert.reinterpretcast added inline comments to D72347: [NFC][XCOFF] Refactor Csect creation into TargetLoweringObjectFile.
Thu, Jan 16, 7:21 AM · Restricted Project
hubert.reinterpretcast added inline comments to D70972: [AIX] Make sure we use export lists for plugins.
Thu, Jan 16, 6:38 AM · Restricted Project

Wed, Jan 15

hubert.reinterpretcast added inline comments to D72479: [PowerPC][AIX] Make PIC the default relocation model for AIX.
Wed, Jan 15, 8:34 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72479: [PowerPC][AIX] Make PIC the default relocation model for AIX.
Wed, Jan 15, 8:31 AM · Restricted Project
hubert.reinterpretcast committed rG63b428e3861b: DWARFDebugLine.cpp: Format unknown line number standard opcodes (authored by hubert.reinterpretcast).
DWARFDebugLine.cpp: Format unknown line number standard opcodes
Wed, Jan 15, 7:48 AM
hubert.reinterpretcast closed D72369: DWARFDebugLine.cpp: Format unknown line number standard opcodes.
Wed, Jan 15, 7:47 AM · Restricted Project
hubert.reinterpretcast committed rGe429f24ed8b1: [CMake] Enable -qfuncsect when building with IBM XL (authored by hubert.reinterpretcast).
[CMake] Enable -qfuncsect when building with IBM XL
Wed, Jan 15, 7:47 AM
hubert.reinterpretcast closed D72335: [CMake] Enable -qfuncsect when building with IBM XL.
Wed, Jan 15, 7:47 AM · Restricted Project, Restricted Project
hubert.reinterpretcast retitled D72369: DWARFDebugLine.cpp: Format unknown line number standard opcodes from DWARFDebugLine.cpp: Format NULL as "(null)" to avoid UB to DWARFDebugLine.cpp: Format unknown line number standard opcodes.
Wed, Jan 15, 7:47 AM · Restricted Project

Tue, Jan 14

hubert.reinterpretcast committed rGaca3e70d2bc0: DWARFDebugLine.cpp: Restore LF line endings (authored by hubert.reinterpretcast).
DWARFDebugLine.cpp: Restore LF line endings
Tue, Jan 14, 6:29 PM
hubert.reinterpretcast created D72745: [MC][test] Fix non-portable GNU diff option.
Tue, Jan 14, 5:51 PM · Restricted Project
hubert.reinterpretcast added a comment to D72479: [PowerPC][AIX] Make PIC the default relocation model for AIX.

@stevewan, can you confirm if there are other tools/utilities that need similar treatment?

Tue, Jan 14, 5:41 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72736: [AIX] Add improved interface for retrieving load module paths.
Tue, Jan 14, 4:12 PM · Restricted Project, Restricted Project
hubert.reinterpretcast added inline comments to D72736: [AIX] Add improved interface for retrieving load module paths.
Tue, Jan 14, 3:24 PM · Restricted Project, Restricted Project
hubert.reinterpretcast added a comment to D68115: Zero initialize padding in unions.

So if I understand this the proposal is to have something like -fzero-union-padding which is off by default.
When it's OFF compiler will continue to do whatever it does now.
When it's ON it will set zeroes into padding with or without -ftrivial-auto-var-init.
Is this correct?

Tue, Jan 14, 1:37 PM · Restricted Project
hubert.reinterpretcast accepted D72461: [AIX][XCOFF] Supporting the ReadOnlyWithRel SectionKnd.

LGTM with just the fix to replace the "maybe"s. What we should do for the later TODO can be determined later.

Tue, Jan 14, 8:02 AM · Restricted Project

Mon, Jan 13

hubert.reinterpretcast accepted D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.

LGTM.

Mon, Jan 13, 2:48 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72461: [AIX][XCOFF] Supporting the ReadOnlyWithRel SectionKnd.
Mon, Jan 13, 2:48 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72461: [AIX][XCOFF] Supporting the ReadOnlyWithRel SectionKnd.
Mon, Jan 13, 2:30 PM · Restricted Project
hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Mon, Jan 13, 11:22 AM · Restricted Project

Fri, Jan 10

hubert.reinterpretcast added inline comments to D72479: [PowerPC][AIX] Make PIC the default relocation model for AIX.
Fri, Jan 10, 3:11 PM · Restricted Project

Thu, Jan 9

hubert.reinterpretcast accepted D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.

The test needs some minor changes that can be fixed on the commit. LGTM otherwise.

Thu, Jan 9, 7:03 PM · Restricted Project
hubert.reinterpretcast accepted D71013: [AIX] Allow vararg calls when all arguments reside in registers..

Some minor nits that can be fixed on the commit, but otherwise LGTM.

Thu, Jan 9, 6:44 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72479: [PowerPC][AIX] Make PIC the default relocation model for AIX.
Thu, Jan 9, 2:37 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72461: [AIX][XCOFF] Supporting the ReadOnlyWithRel SectionKnd.
Thu, Jan 9, 9:17 AM · Restricted Project
hubert.reinterpretcast added a comment to D72369: DWARFDebugLine.cpp: Format unknown line number standard opcodes.

I'd still really like to see a test that tests the other code path that tries to print DW_LNS_* (see the inline comment). The obvious thing to me to do would be to update the unit test "ParserPrintsStandardOpcodesWhenRequested", but it would require changes to the DwarfGenerator to create a custom prologue. If you don't want to do this, that's okay, as the code path is already untested.

Thu, Jan 9, 7:53 AM · Restricted Project
hubert.reinterpretcast added a comment to D72369: DWARFDebugLine.cpp: Format unknown line number standard opcodes.

I think a better solution is to print something more meaningful. For example, "DW_LNS_unknown_<value>" or something similar. You can see examples of that with the DW_AT_unknown_* printing in debug-abbrev.s. I don't think that will be significantly more complex and will provide more user-friendly output.

Thu, Jan 9, 7:34 AM · Restricted Project
hubert.reinterpretcast updated the diff for D72369: DWARFDebugLine.cpp: Format unknown line number standard opcodes.

DWARFDebugLine.cpp: Format unknown line number standard opcodes

Thu, Jan 9, 6:56 AM · Restricted Project

Wed, Jan 8

hubert.reinterpretcast added a comment to D65761: Add Windows Control Flow Guard checks (/guard:cf)..

I have confirmed that the case I mentioned fails with rGd157a9b.

Wed, Jan 8, 4:14 PM · Restricted Project, Restricted Project
hubert.reinterpretcast added a comment to D68115: Zero initialize padding in unions.

Does this have to be an unilateral change,
likely penalizing non--ftrivial-auto-var-init= cases,
i.e. [why] can't it be only done for when -ftrivial-auto-var-init= is enabled?

We left off near that conclusion (https://reviews.llvm.org/D68115#1686887); however, -ftrivial-auto-var-init= merely increases the chance that code expecting the zeroing to occur would break. A separate option to control zeroing for union padding would help in cases where the zeroing does not happen for reasons other than -ftrivial-auto-var-init.

Wed, Jan 8, 4:05 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71013: [AIX] Allow vararg calls when all arguments reside in registers..
Wed, Jan 8, 9:25 AM · Restricted Project
hubert.reinterpretcast updated subscribers of D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Wed, Jan 8, 8:09 AM · Restricted Project
hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Wed, Jan 8, 7:40 AM · Restricted Project
hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Wed, Jan 8, 7:40 AM · Restricted Project

Tue, Jan 7

hubert.reinterpretcast added a comment to D71144: [AIX] Use csect reference for function address constants.

@daltenty, following up on needing to set linkage for csects in the assembly (in a more general case than just for function descriptors, as you pointed out): We are missing the linkage on the assembly path for weak references.

Tue, Jan 7, 3:51 PM · Restricted Project
hubert.reinterpretcast created D72369: DWARFDebugLine.cpp: Format unknown line number standard opcodes.
Tue, Jan 7, 3:23 PM · Restricted Project
hubert.reinterpretcast added a comment to D71144: [AIX] Use csect reference for function address constants.

@jasonliu, @daltenty; as discussed off-list today, it would be much cleaner if we dropped producing a label definition for the function descriptor. In particular, I have validated my suspicions that referencing the DS csect in a file where the function is defined weak is problematic if the linkage is associated with the label definition and not the csect. Thank you both for tracking this on your TODO lists.

Tue, Jan 7, 2:45 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72027: [XCOFF][AIX] Support basic relocation type on AIX.
Tue, Jan 7, 8:58 AM · Restricted Project
hubert.reinterpretcast requested changes to D72027: [XCOFF][AIX] Support basic relocation type on AIX.
Tue, Jan 7, 8:49 AM · Restricted Project
hubert.reinterpretcast created D72335: [CMake] Enable -qfuncsect when building with IBM XL.
Tue, Jan 7, 7:24 AM · Restricted Project, Restricted Project

Mon, Jan 6

hubert.reinterpretcast added inline comments to D72027: [XCOFF][AIX] Support basic relocation type on AIX.
Mon, Jan 6, 9:57 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72027: [XCOFF][AIX] Support basic relocation type on AIX.
Mon, Jan 6, 8:33 PM · Restricted Project
hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Mon, Jan 6, 3:03 PM · Restricted Project
hubert.reinterpretcast updated the summary of D71551: [AIX][XCOFF]Implement mergeable const.
Mon, Jan 6, 2:44 PM · Restricted Project
hubert.reinterpretcast accepted D71551: [AIX][XCOFF]Implement mergeable const.

LGTM.

Mon, Jan 6, 2:35 PM · Restricted Project
hubert.reinterpretcast added inline comments to D72027: [XCOFF][AIX] Support basic relocation type on AIX.
Mon, Jan 6, 2:25 PM · Restricted Project

Sat, Jan 4

hubert.reinterpretcast added inline comments to D71916: High-Level Code-Review Documentation Update.
Sat, Jan 4, 8:45 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71916: High-Level Code-Review Documentation Update.
Sat, Jan 4, 7:51 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71845: [AIX][XCOFF] add test for raw text section content and test section header.
Sat, Jan 4, 6:47 PM · Restricted Project
hubert.reinterpretcast added inline comments to D70972: [AIX] Make sure we use export lists for plugins.
Sat, Jan 4, 2:14 PM · Restricted Project
hubert.reinterpretcast added a comment to D65761: Add Windows Control Flow Guard checks (/guard:cf)..

Is building check-all immediately after configuration expected to be clean? I have a build configured to build LLVM with just clang and just with the PowerPC target, and it seems check-all (under such a configuration) does not trigger the building of the library added in this patch; which, in turn, leads to test failures.

Sat, Jan 4, 10:43 AM · Restricted Project, Restricted Project

Thu, Jan 2

hubert.reinterpretcast added inline comments to D71551: [AIX][XCOFF]Implement mergeable const.
Thu, Jan 2, 10:08 AM · Restricted Project
hubert.reinterpretcast added inline comments to D71916: High-Level Code-Review Documentation Update.
Thu, Jan 2, 9:10 AM · Restricted Project

Mon, Dec 23

hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Mon, Dec 23, 8:10 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71551: [AIX][XCOFF]Implement mergeable const.
Mon, Dec 23, 10:38 AM · Restricted Project

Dec 20 2019

hubert.reinterpretcast accepted D71504: [PowerPC] Enable sret arguments..

I think if the front-end AIX support was further along then a patch like this should be deferred until we fixed (and verified) the IR the front-end generates for sret arguments. With how clang/CodeGen is going through the SVR4 targets for AIX its safe to consider any Clang AIX support as extremely experimental right now, and the back-end implementation shouldn't be held up based on Clang support for the target.

Dec 20 2019, 6:30 PM · Restricted Project
hubert.reinterpretcast added a comment to D71504: [PowerPC] Enable sret arguments..

@sfertile, I'm not seeing the change to the test to add -verify-machineinstrs. Perhaps you have a newer version to upload?

Dec 20 2019, 12:34 PM · Restricted Project

Dec 19 2019

hubert.reinterpretcast added a comment to D71144: [AIX] Use csect reference for function address constants.

Just one minor issue with the patch itself, and otherwise, I don't see an issue with the patch in isolation.

Dec 19 2019, 2:04 PM · Restricted Project

Dec 18 2019

hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Dec 18 2019, 6:37 PM · Restricted Project
hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Dec 18 2019, 5:59 PM · Restricted Project
hubert.reinterpretcast added a comment to D71667: [XCOFF][AIX] Fix for missing of undefined symbols from symbol table.

Looks fine here as well. Not sure if other reviewers would have comments though.

Dec 18 2019, 12:43 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71551: [AIX][XCOFF]Implement mergeable const.
Dec 18 2019, 12:14 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71144: [AIX] Use csect reference for function address constants.
Dec 18 2019, 12:14 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71144: [AIX] Use csect reference for function address constants.
Dec 18 2019, 11:45 AM · Restricted Project

Dec 17 2019

hubert.reinterpretcast added inline comments to D71144: [AIX] Use csect reference for function address constants.
Dec 17 2019, 3:15 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71504: [PowerPC] Enable sret arguments..
Dec 17 2019, 2:26 PM · Restricted Project
hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Dec 17 2019, 9:35 AM · Restricted Project
hubert.reinterpretcast added a comment to D71504: [PowerPC] Enable sret arguments..

I don't know what happens in the case where the front-end doesn't handle some cases correctly but the back-end does. Is there a danger in enabling this in the back-end knowing we don't get correct IR for complex types for AIX? Is there an error in the front-end for those before we get to this step?

There's no error from the front-end. In terms of continuous delivery, this leads to silently bad code that could lead to incorrect output or crashes at runtime.

Dec 17 2019, 7:18 AM · Restricted Project

Dec 16 2019

hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Dec 16 2019, 9:04 PM · Restricted Project
hubert.reinterpretcast accepted D71125: [AIX] Avoid unset csect assert for functions defined after their use in TOC.

@daltenty, please update the commit message to indicate the corrected scope. This LGTM insofar as it fits the current handling of function descriptors for which we have a definition in the current object.

Dec 16 2019, 3:47 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71144: [AIX] Use csect reference for function address constants.
Dec 16 2019, 3:18 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71551: [AIX][XCOFF]Implement mergeable const.
Dec 16 2019, 12:07 PM · Restricted Project
hubert.reinterpretcast added a comment to D71504: [PowerPC] Enable sret arguments..

So it doesn't look like we need to add any special handling for struct returns on AIX? I had a quick look and none of the other PPC targets seem to have anything.

Dec 16 2019, 9:20 AM · Restricted Project
hubert.reinterpretcast added inline comments to D71013: [AIX] Allow vararg calls when all arguments reside in registers..
Dec 16 2019, 8:43 AM · Restricted Project

Dec 14 2019

hubert.reinterpretcast added inline comments to D71013: [AIX] Allow vararg calls when all arguments reside in registers..
Dec 14 2019, 8:14 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71504: [PowerPC] Enable sret arguments..
Dec 14 2019, 7:16 PM · Restricted Project
hubert.reinterpretcast requested changes to D71125: [AIX] Avoid unset csect assert for functions defined after their use in TOC.

The scope of the "drive-by" change is heading into redesign territory.

Dec 14 2019, 5:45 PM · Restricted Project

Dec 13 2019

hubert.reinterpretcast added inline comments to D71144: [AIX] Use csect reference for function address constants.
Dec 13 2019, 9:40 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71013: [AIX] Allow vararg calls when all arguments reside in registers..
Dec 13 2019, 9:27 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71119: [NFC][AIX][XCOFF] fixed compile warning on the strncpy..
Dec 13 2019, 7:33 PM · Restricted Project
hubert.reinterpretcast closed D70724: [PowerPC] Add Support for indirect calls on AIX..

Committed as rG93faa237da8d. @sfertile, typo in the commit message: s/Differtential/Differential/;

Dec 13 2019, 7:24 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71013: [AIX] Allow vararg calls when all arguments reside in registers..
Dec 13 2019, 8:15 AM · Restricted Project

Dec 12 2019

hubert.reinterpretcast added inline comments to D71013: [AIX] Allow vararg calls when all arguments reside in registers..
Dec 12 2019, 4:40 PM · Restricted Project
hubert.reinterpretcast added inline comments to D71144: [AIX] Use csect reference for function address constants.
Dec 12 2019, 3:53 PM · Restricted Project
hubert.reinterpretcast added a comment to D71144: [AIX] Use csect reference for function address constants.

The commit subject should be more direct in indicating what the patch does, not what problem it solves.

Dec 12 2019, 3:35 PM · Restricted Project

Dec 11 2019

hubert.reinterpretcast accepted D70724: [PowerPC] Add Support for indirect calls on AIX..

Nit: It may be better to rename the test aix-indirect-call.ll to be consistent.

I agree.

Dec 11 2019, 1:05 PM · Restricted Project
hubert.reinterpretcast added inline comments to D70724: [PowerPC] Add Support for indirect calls on AIX..
Dec 11 2019, 9:54 AM · Restricted Project

Dec 10 2019

hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Dec 10 2019, 8:24 AM · Restricted Project
hubert.reinterpretcast added inline comments to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.
Dec 10 2019, 8:19 AM · Restricted Project
hubert.reinterpretcast added a comment to D71125: [AIX] Avoid unset csect assert for functions defined after their use in TOC.

This patch might need splitting. At least the commit subject should encompass the scope more. Once the tests are added for weak external or internal linkage, the "drive-by" change will be sizable.

Dec 10 2019, 7:33 AM · Restricted Project

Dec 9 2019

hubert.reinterpretcast added a comment to D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove.

As Jason suggested, the code duplication can be reduced:

const auto ProduceDescriptorAndReplaceCallee =
    [&](StringRef FuncName, bool EnsureContainingCsectIsSet,
        const auto &GetStorageClass) {
      auto &Context = DAG.getMachineFunction().getMMI().getContext();
Dec 9 2019, 8:57 PM · Restricted Project
hubert.reinterpretcast added a comment to D70724: [PowerPC] Add Support for indirect calls on AIX..

Some minor comments; otherwise, I think this is fine, but I don't know if others believe there are still comments to be addressed.

Dec 9 2019, 7:00 PM · Restricted Project