Page MenuHomePhabricator

shiva0217 (Shiva Chen)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 25 2016, 5:14 PM (138 w, 5 d)

Recent Activity

Wed, Aug 21

shiva0217 committed rG72a41e7b0d04: [TargetLowering] Remove optional arguments passing to makeLibCall (authored by shiva0217).
[TargetLowering] Remove optional arguments passing to makeLibCall
Wed, Aug 21, 10:02 PM

Tue, Aug 20

shiva0217 added a comment to D66278: [RISCV] Enable tail call opt for variadic function.

Hi Jim,

Tue, Aug 20, 8:01 PM · Restricted Project
shiva0217 updated the diff for D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

Add comment for the new field in MakeLibCallOptions struct.

Tue, Aug 20, 6:41 PM · Restricted Project

Mon, Aug 19

shiva0217 added inline comments to D62190: [RISCV] Allow shrink wrapping for RISC-V.
Mon, Aug 19, 11:25 PM · Restricted Project
shiva0217 updated the diff for D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

Update patch to reflect the comments.

Mon, Aug 19, 7:45 PM · Restricted Project
shiva0217 added inline comments to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.
Mon, Aug 19, 7:35 PM · Restricted Project

Thu, Aug 8

shiva0217 updated the diff for D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

I remove the IsCastFromFloat flag and store the SDNode before soften in MakeLibCallOptions struct. So that the shouldExtendTypeInLibCall could get the original type directly and get rid of the complicate IsCastFromFloat setting method. I try to illustrate most of the single soft-float functions in rv64i-single-softfloat.ll. Please kindly remind me if I still missing something, thanks.

Thu, Aug 8, 11:49 PM · Restricted Project
shiva0217 updated the diff for D65795: [TargetLowering] Remove optional arguments passing to makeLibCall.

Only define the bits will be used for makelibCall in MakeLibCallOptions struct.

Thu, Aug 8, 11:33 PM · Restricted Project
shiva0217 added inline comments to D65795: [TargetLowering] Remove optional arguments passing to makeLibCall.
Thu, Aug 8, 11:25 PM · Restricted Project

Wed, Aug 7

shiva0217 updated the diff for D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

Rebase the patch base on D65795 and update test cases.
Thanks for Eli's comments and Alex's excellent floating test cases that we could easily observe the difference.

Wed, Aug 7, 1:17 AM · Restricted Project
shiva0217 added inline comments to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.
Wed, Aug 7, 1:06 AM · Restricted Project

Tue, Aug 6

shiva0217 added inline comments to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.
Tue, Aug 6, 2:31 AM · Restricted Project
shiva0217 added a parent revision for D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall: D65795: [TargetLowering] Remove optional arguments passing to makeLibCall.
Tue, Aug 6, 2:18 AM · Restricted Project
shiva0217 added a child revision for D65795: [TargetLowering] Remove optional arguments passing to makeLibCall: D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.
Tue, Aug 6, 2:18 AM · Restricted Project
shiva0217 created D65795: [TargetLowering] Remove optional arguments passing to makeLibCall.
Tue, Aug 6, 2:18 AM · Restricted Project

Mon, Aug 5

shiva0217 committed rGb12056bd3390: [RISCV] Custom legalize i32 operations for RV64 to reduce signed extensions (authored by shiva0217).
[RISCV] Custom legalize i32 operations for RV64 to reduce signed extensions
Mon, Aug 5, 5:24 PM

Thu, Aug 1

shiva0217 updated the diff for D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

Update the patch to catch divsf3 case.

Thu, Aug 1, 7:13 PM · Restricted Project
shiva0217 added inline comments to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.
Thu, Aug 1, 7:11 PM · Restricted Project
shiva0217 updated the diff for D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

Thanks for pointing me the right direction.
I added shouldExtendTypeInLibCall target hook to indicate the Libcall should do the extension or not.
The parameter IsCastFromFloat will be passed to TargetLowering::makeLibCall and shouldExtendTypeInLibCall. So shouldExtendTypeInLibCall can identify the Type is casting by floating and disable the extensions.
Without generating AssertZext for the Libcall return value, and operation for clearing upper bits will be preserved.

Thu, Aug 1, 1:17 AM · Restricted Project

Wed, Jul 31

shiva0217 added a comment to D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.

I thought the problem was in that in the complex add case, we needed to zero-extend the i32 into an i64, in order to zero the upper 32 bits. This patch seems to sign-extend the libcall instead. Is there a shouldZeroExtendTypeInLibCall callback?

Wed, Jul 31, 1:13 AM · Restricted Project

Tue, Jul 30

shiva0217 created D65497: [RISCV] Avoid generating AssertZext for LP64 ABI when lowering floating Libcall.
Tue, Jul 30, 10:45 PM · Restricted Project
shiva0217 updated the diff for D65434: [RISCV] Custom legalize i32 operations for RV64 to reduce signed extensions.

Updated the test cases changed by the patch.

Tue, Jul 30, 6:40 PM · Restricted Project
shiva0217 created D65434: [RISCV] Custom legalize i32 operations for RV64 to reduce signed extensions.
Tue, Jul 30, 1:46 AM · Restricted Project

Jun 18 2019

shiva0217 added inline comments to D62592: [RISCV] Add support for RVC HINT instructions.
Jun 18 2019, 11:46 PM · Restricted Project

Jun 4 2019

shiva0217 added inline comments to D62592: [RISCV] Add support for RVC HINT instructions.
Jun 4 2019, 10:49 PM · Restricted Project

May 28 2019

shiva0217 added a comment to D61773: [RISCV] Add CFI directives for RISCV prologue/epilog..

Hi Kai,
We could also generate .cfi_restore for epilogue by:

unsigned CFIIndex = MF->addFrameInst(MCCFIInstruction::createRestore(
    nullptr, MRI->getDwarfRegNum(Reg, 1)));
BuildMI(MBB, MBBI, DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
    .addCFIIndex(CFIIndex);
May 28 2019, 1:20 AM · Restricted Project

May 14 2019

shiva0217 created D61884: [RISCV] Support stack offset exceed 32-bit for RV64.
May 14 2019, 1:50 AM · Restricted Project
shiva0217 added a comment to D61773: [RISCV] Add CFI directives for RISCV prologue/epilog..

Hi Kai,

May 14 2019, 1:46 AM · Restricted Project

May 9 2019

shiva0217 added a comment to D61665: [TailCall] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address.
In D61665#1496085, @asb wrote:

Yes, I think leaving the current behaviour probably makes sense. I encountered that test failure too and mask that test on the basis that that those builtins aren't documented as being guaranteed to work https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html (that page even notes that crashing is allowable behaviour).

May 9 2019, 12:14 AM · Restricted Project

May 8 2019

shiva0217 abandoned D61626: [RISCV] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address.
May 8 2019, 6:56 PM · Restricted Project
shiva0217 abandoned D61665: [TailCall] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address.

I have a few concerns here:

  1. Looping over every instruction in a function is expensive, and makes any pass which checks this for every call in a function take quadratic time overall.

It may too expansive, using function attribute as Hal's comment would be a better approach.

  1. You can't inspect the body of a function pointer, or a function in a different translation unit, so we can't make this work consistently.

Yes, we can't detect the function in a different translation unit or pointed by a function pointer.

  1. Even in the same translation unit, how do we "preserve" the behavior for values greater than 1?

Yes, disabling tail call can only preserve the stack in depth 1, it may have other optimizations change the behavior of the depth greater than one.


I'd prefer to just leave the current behavior if it isn't causing any practical problems. The user can always use -fno-optimize-sibling-calls if their codebase needs it for some reason.

If we do wish to make our "best effort" contain more effort, I think that we'd want to do this during function-attribute inference - there we can iterate over the call graph and add some inhibiting attributes. That having been said, if the only use case we have for this is matching some portion of GCC's heuristic for the purpose of making their test case pass, I'm not sure that this is worthwhile.

May 8 2019, 6:51 PM · Restricted Project
shiva0217 added a comment to D61665: [TailCall] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address.
In D61665#1494592, @asb wrote:

The description of the llvm.frameaddress and llvm.returnaddress intrinsics seems to indicate that these are "best effort" and LLVM doesn't really guarantee a correct result for a depth > 1 https://llvm.org/docs/LangRef.html#llvm-returnaddress-intrinsic https://llvm.org/docs/LangRef.html#llvm-returnaddress-intrinsic

Is there a particular use case that is improved by improving the quality of frameaddress/returnaddress results?

May 8 2019, 5:16 AM · Restricted Project

May 7 2019

shiva0217 added a comment to D61626: [RISCV] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address.

Is this really a target-independent problem?

May 7 2019, 8:12 PM · Restricted Project
shiva0217 created D61665: [TailCall] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address.
May 7 2019, 8:03 PM · Restricted Project

May 6 2019

shiva0217 created D61626: [RISCV] Disable tail call if the callee function contain __builtin_frame_address or __builtin_return_address.
May 6 2019, 11:09 PM · Restricted Project

Apr 10 2019

shiva0217 committed rG7cc03bd06487: [RISCV] Put data smaller than eight bytes to small data section (authored by shiva0217).
[RISCV] Put data smaller than eight bytes to small data section
Apr 10 2019, 9:58 PM
shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Update patch to address Alex's comments.
Hi Alex, thanks for the review.

Apr 10 2019, 6:51 PM · Restricted Project

Mar 31 2019

shiva0217 accepted D59686: [RISCV] Don't evaluatePCRelLo if a relocation will be forced (e.g. due to linker relaxation).

Thanks for the update, LGTM.

Mar 31 2019, 7:19 PM · Restricted Project

Mar 29 2019

shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Update patch to address the comments from Eli and Ana.

Mar 29 2019, 2:39 AM · Restricted Project
shiva0217 added inline comments to D57493: [RISCV] Put data smaller than eight bytes to small data section.
Mar 29 2019, 2:34 AM · Restricted Project

Mar 28 2019

shiva0217 added inline comments to D57497: [RISCV] Passing small data limitation value to RISCV backend.
Mar 28 2019, 6:39 AM · Restricted Project
shiva0217 updated the diff for D57497: [RISCV] Passing small data limitation value to RISCV backend.

Add warning message for -msmall-data-limit with -fpic or RV64 with -mcmodel=large

Mar 28 2019, 5:53 AM · Restricted Project

Mar 22 2019

shiva0217 added a comment to D59686: [RISCV] Don't evaluatePCRelLo if a relocation will be forced (e.g. due to linker relaxation).

Could you add a case in option-relax.s after .option norelax to indicate the relocation type for local symbol will leave once the relaxation has been enabled?

Mar 22 2019, 7:34 AM · Restricted Project

Mar 20 2019

shiva0217 updated the diff for D57497: [RISCV] Passing small data limitation value to RISCV backend.

Update patch to address Ana's comments.

  1. Ignoring -G when -msmall-data-limit= in the command line and show the warning message
  2. Removing LTO plugin invoking which should introduce in another patch
Mar 20 2019, 11:49 PM · Restricted Project
shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Remove riscv-ssection-threshold flag because front end will pass the threshold by module flag.

Mar 20 2019, 11:42 PM · Restricted Project
shiva0217 added inline comments to D57493: [RISCV] Put data smaller than eight bytes to small data section.
Mar 20 2019, 11:32 PM · Restricted Project

Mar 14 2019

shiva0217 updated the diff for D57497: [RISCV] Passing small data limitation value to RISCV backend.

Passing small data limitation by module flag.

Mar 14 2019, 4:09 AM · Restricted Project
shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Add getModuleMetadata() to read SmallDataLimit Module flag.

Mar 14 2019, 4:09 AM · Restricted Project

Feb 19 2019

shiva0217 added a comment to D57497: [RISCV] Passing small data limitation value to RISCV backend.

Did you mean declare as a target feature in RISCV.td or I misunderstanding something?

That's sort of the right idea, but I don't think it works in this context because we aren't trying to change the generated code for a function; we actually need to stick the global into a specific section. Maybe worth sending an email to llvmdev to discuss the right way to represent this in IR?

Hi Eli,
I have sent the email to llvmdev http://lists.llvm.org/pipermail/llvm-dev/2019-February/130222.html. It seems that there're not much consensus on how to represent in IR. I incline to implement passing through -plugin-opt= as the first version. We could create an incremental patch when we have more consensus on IR approach. What do you think?

Feb 19 2019, 6:41 PM · Restricted Project

Feb 17 2019

shiva0217 committed rGe3aaeabb6da6: [RISCV] Default enable RISCV linker relaxation (authored by shiva0217).
[RISCV] Default enable RISCV linker relaxation
Feb 17 2019, 8:06 AM
shiva0217 updated the diff for D47127: [RISCV] Default enable RISCV linker relaxation.

Update patch to address Alex's comment.

Feb 17 2019, 7:44 AM · Restricted Project

Feb 11 2019

shiva0217 added a comment to D57497: [RISCV] Passing small data limitation value to RISCV backend.

If this is a target flag in GCC, shouldn't we make it a LLVM Target feature and pass it as -mattr, just like done for mrelax?

Feb 11 2019, 6:21 PM · Restricted Project

Feb 5 2019

shiva0217 updated the diff for D57497: [RISCV] Passing small data limitation value to RISCV backend.
  1. Remove passing path for LTO because Eli raised the concern that whether it would appropriate to assign the same limitation for all files when LTO enabled.
  2. Add -msmall-data-limitation= as James pointed out it's the main setting flag for RISCV GCC
Feb 5 2019, 11:33 PM · Restricted Project

Feb 3 2019

shiva0217 added a comment to D57497: [RISCV] Passing small data limitation value to RISCV backend.

I don't see -plugin-opt=-riscv-ssection-threshold=.. being passed.
tools::gnutools::Linker::ConstructJob is being invoked with target riscv32-unknown-linux-gnu
It has to work for riscv32-unknown-linux-gnu and riscv32-unknown-elf

Feb 3 2019, 6:52 PM · Restricted Project
shiva0217 updated the diff for D57497: [RISCV] Passing small data limitation value to RISCV backend.

Support passing small data limitation for target riscv32-unknown-linux-gnu with LTO enabled.

Feb 3 2019, 6:44 PM · Restricted Project
shiva0217 added a comment to D57497: [RISCV] Passing small data limitation value to RISCV backend.

Hi Shiva, I think you need to check for and pass along the -G option to the linker (gnutools::Linker and RISCV::Linker) and will be available for LTO. Check Hexagon, it passes the threshold value to the assembler (via -gpsize) and linker (via -G).

Feb 3 2019, 1:59 AM · Restricted Project
shiva0217 updated the diff for D57497: [RISCV] Passing small data limitation value to RISCV backend.
  1. Setting small data limitation to zero for PIC and RV64 with large code model.
  2. Support passing small data limitation with LTO enabled.
Feb 3 2019, 1:39 AM · Restricted Project
shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Remove guard condition for large code model which will implement in clang.

Feb 3 2019, 1:31 AM · Restricted Project

Feb 2 2019

shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Add testing for large code model.

Feb 2 2019, 5:45 PM · Restricted Project

Feb 1 2019

shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Correct the default small data limitation to 8 bytes for RV32 and RV64.
Setting the limitation to 0 for PIC.
The test case didn' add the testing line for PIC because we don't support pic yet, it will trigger "Unable to lowerGlobalAddress" error message.

Feb 1 2019, 9:40 PM · Restricted Project
shiva0217 added inline comments to D57493: [RISCV] Put data smaller than eight bytes to small data section.
Feb 1 2019, 5:37 PM · Restricted Project
shiva0217 updated the diff for D57493: [RISCV] Put data smaller than eight bytes to small data section.

Fix the issue for -G0 as Ana pointed out.

Feb 1 2019, 5:33 PM · Restricted Project

Jan 31 2019

Herald added a project to D57497: [RISCV] Passing small data limitation value to RISCV backend: Restricted Project.

As this mllvm option only affects the creation of ELF objects, do we also need to add a similar option for the LTO case, as the -G value would have no effect otherwise?

Jan 31 2019, 7:28 PM · Restricted Project

Jan 30 2019

shiva0217 added a child revision for D57493: [RISCV] Put data smaller than eight bytes to small data section: D57497: [RISCV] Passing small data limitation value to RISCV backend.
Jan 30 2019, 9:51 PM · Restricted Project
shiva0217 added a parent revision for D57497: [RISCV] Passing small data limitation value to RISCV backend: D57493: [RISCV] Put data smaller than eight bytes to small data section.
Jan 30 2019, 9:51 PM · Restricted Project
shiva0217 created D57497: [RISCV] Passing small data limitation value to RISCV backend.
Jan 30 2019, 9:48 PM · Restricted Project
shiva0217 created D57493: [RISCV] Put data smaller than eight bytes to small data section.
Jan 30 2019, 6:30 PM · Restricted Project

Jan 18 2019

shiva0217 closed D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

Hi Eli, Thanks for the review.

Jan 18 2019, 12:55 AM

Jan 17 2019

shiva0217 updated the diff for D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.

Hi @asb, I got your point, we could guard the target hook with AF.hasEmitNops(), so the behavior of .align 4, 1 will be the same as gnu assembler. What do you think?

Jan 17 2019, 10:48 PM

Jan 11 2019

shiva0217 added a comment to D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.
In D47755#1352910, @asb wrote:

Hi Alex, I have added a constant pool test case at the end of align.s to verify the padding behavior for constant pool. Once D45961 landing and changing the behavior, we should be able to aware. It seems that GCC will insert Nops for padding constant pool. Could you illustrate more about "use .align with a value to fill with"? I could test on GCC.

I meant that .align/.p2align in GNU as accept a padding value https://sourceware.org/binutils/docs/as/Align.html#Align. It seems that for RISC-V, it accepts and uses this padding value even when relaxation is enabled, but won't emit the R_RISCV_RELAX in that case (which I suppose makes sense, as the linker wouldn't respect the padding value). I'm not saying we should definitely replicate GNU as here - obviously performing alignment without generating R_RISCV_RELAX is quite liable to break things unexpectedly. But it might not be a bad starting point if it's easy to support. Either way, we should have tests that demonsrate how we handle those cases.

Jan 11 2019, 8:16 PM
shiva0217 updated the diff for D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.
  1. Add test case for alignment directive with a specific padding value
  2. Rename the new target hooks to NopBytesToInsertForAlignDirectiveInCodeSection and insertFixupForAlignDirectiveInCodeSection
Jan 11 2019, 7:03 PM

Jan 7 2019

shiva0217 updated the diff for D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.

Update patch to reflect James comments.

Jan 7 2019, 11:06 PM
shiva0217 added inline comments to D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.
Jan 7 2019, 11:05 PM

Jan 6 2019

shiva0217 added a comment to D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.

Ping?

Jan 6 2019, 9:21 PM

Jan 3 2019

shiva0217 accepted D52298: [RISCV][MC] Add support for evaluating constant symbols as immediates.
Jan 3 2019, 9:53 PM
shiva0217 added a comment to D52298: [RISCV][MC] Add support for evaluating constant symbols as immediates.

Hi Alex,
Thanks for the updates, looks good to me.

Jan 3 2019, 9:50 PM

Dec 18 2018

shiva0217 added a comment to D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.

Hi Alex, I have added a constant pool test case at the end of align.s to verify the padding behavior for constant pool. Once D45961 landing and changing the behavior, we should be able to aware. It seems that GCC will insert Nops for padding constant pool. Could you illustrate more about "use .align with a value to fill with"? I could test on GCC.

Dec 18 2018, 12:13 AM

Dec 17 2018

shiva0217 updated the diff for D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled.

Rebase and rename insertNopBytesForAlignDirectiveInTextSection to NopBytesToInsertForAlignDirectiveInTextSection

Dec 17 2018, 11:51 PM

Dec 7 2018

shiva0217 added a comment to D42374: [RFC] Add IsFixed field to ISD::ArgFlagsTy.

Hi Alex,
I think it's great that the target description file could determine the argument is fixed or not. In this way, the targets have different ABI behavior for non-fixed arguments could describe by TD files without extra custom c++ codes. It seems that Mips and RISCV already have variable argument test cases to cover the changed. Do other targets involve the changed have variable argument test case? If it's not, could you add the test cases to cover the changed?

Dec 7 2018, 12:52 AM

Dec 4 2018

shiva0217 abandoned D55253: [RISCV] Fix incorrect use of MCInstBuilder in RISCVMCCodeEmitter.
Dec 4 2018, 12:59 AM
shiva0217 added a comment to D55253: [RISCV] Fix incorrect use of MCInstBuilder in RISCVMCCodeEmitter.

Hi @rogfer01, got it. Thanks to pointing out the difference. I'll drop the patch.

Dec 4 2018, 12:58 AM

Dec 3 2018

shiva0217 created D55253: [RISCV] Fix incorrect use of MCInstBuilder in RISCVMCCodeEmitter.
Dec 3 2018, 11:30 PM

Nov 22 2018

shiva0217 added a comment to D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

Hi @TimNN,
Thanks for the test case, that really help me a lot.

Nov 22 2018, 12:04 AM

Nov 21 2018

shiva0217 updated the diff for D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

Add the test case provided by Tim.

Nov 21 2018, 11:56 PM

Nov 20 2018

shiva0217 added a comment to D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

Ping?

Nov 20 2018, 11:27 PM

Nov 3 2018

shiva0217 added a comment to D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

Hi @samparker,
Thanks for your time to take a look at the code.

Nov 3 2018, 2:31 AM

Nov 2 2018

shiva0217 added reviewers for D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent: eli.friedman, samparker.
Nov 2 2018, 1:35 AM
shiva0217 updated subscribers of D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

I look into the log history of ScheduleDAGRRList.cpp, it seems that @eli.friedman and @samparker have been fixed some issue relative to CallResource.
I'll add them as the reviewers. Hopefully, they could have time to review our patch and won't bother them too much.

Nov 2 2018, 1:35 AM

Oct 22 2018

shiva0217 retitled D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent from [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN to [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.
Oct 22 2018, 11:06 PM
shiva0217 added a comment to D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

What about MachineVerifier::verifyStackFrame()? That currently rejects any function with nested callframe setups. You will probably see your testcases fail when running with -verify-machineinstrs.

Oct 22 2018, 11:05 PM
shiva0217 updated the diff for D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

Update patch to address Tim's comments.

Oct 22 2018, 8:16 PM
shiva0217 added inline comments to D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.
Oct 22 2018, 8:16 PM
shiva0217 retitled D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent from [ScheduleDAGRRList] Do not preschedule ADJCALLSTACKDOWN to [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN.
Oct 22 2018, 8:14 PM
shiva0217 added a comment to D53230: [RISCV] Introduce codegen patterns for RV64M-only instructions.

Probably need to add

def : Pat<(sext_inreg (sdiv GPR:$rs1, GPR:$rs2), i32),
          (DIVW GPR:$rs1, GPR:$rs2)>;
def : Pat<(sext_inreg (udiv GPR:$rs1, GPR:$rs2), i32),
          (DIVUW GPR:$rs1, GPR:$rs2)>;

Because DAG combine may create these patterns.

Oct 22 2018, 2:04 AM

Oct 21 2018

shiva0217 created D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.
Oct 21 2018, 10:56 PM

Sep 26 2018

shiva0217 added a comment to D52299: [RISCV][MC] Accept %lo and %pcrel_lo on operands to li.

LGTM.

Sep 26 2018, 1:12 AM
shiva0217 added a comment to D52298: [RISCV][MC] Add support for evaluating constant symbols as immediates.

Hi Alex, another solution could be calling parseExpression for AsmToken::Identifier in RISCVAsmParser::parseImmediate instead of calling parseIdentifier.

case AsmToken::String:
case AsmToken::Identifier: {
  if (getParser().parseExpression(Res))
    return MatchOperand_ParseFail;
  break;
}

parseExpression will call parsePrimaryExpr which contain the logical to call parseIdentifier and also try to get the symbol value when the symbol is variable.
What do you think?

Sep 26 2018, 12:53 AM

Aug 20 2018

shiva0217 added inline comments to D46423: [RISCV] Support .option relax and .option norelax.
Aug 20 2018, 7:04 PM
shiva0217 added a comment to D46423: [RISCV] Support .option relax and .option norelax.

Apology. I thought the assembler will parse all the instructions and then do the encoding which is incorrect. It will parse and encode one instruction at a time. So we don't need using MCinst's Flag to record the instruction state and NoRelaxLocStart shouldn't need.
We could define FeatureNoRelaxREL feature to indicate the suppression of R_RISCV_RELAX relocation type instead of clear FeatureRelax bit.
Because once the .option relax occur in the file, shouldForceRelocation, and requiresDiffExpressionRelocations should return true to make sure the local branches and label differences will leave the relocation types.
With the relocation types, the linker relaxation could do the right adjustment when the code size changed.
I think it's quite similar to the approach @simoncook mention in https://reviews.llvm.org/D45181#1086541.
Is it a feasible way to implement? or we may still break something in certain cases?

Aug 20 2018, 8:09 AM
shiva0217 added a comment to D46423: [RISCV] Support .option relax and .option norelax.

We may need to find a way to indicate the .option norelax effective scope to handle the following case.

.option norelax
 call f1
.option relax
 call f2
.option norelax
 call f3

In this case, only call f2 should insert R_RISCV_RELAX relocation type.

Aug 20 2018, 2:23 AM