This is an archive of the discontinued LLVM Phabricator instance.

[PATCH 2/2] [x86] Add support for "probe-stack"
Needs ReviewPublic

Authored by Zoxc on May 10 2015, 9:22 PM.

Details

Reviewers
majnemer
Summary

Adds support for "probe-stack" on x86

Diff Detail

Event Timeline

Zoxc updated this revision to Diff 25440.May 10 2015, 9:22 PM
Zoxc retitled this revision from to [PATCH 2/2] [x86] Add support for "probe-stack".
Zoxc updated this object.
Zoxc edited the test plan for this revision. (Show Details)
Zoxc added a reviewer: majnemer.
Zoxc set the repository for this revision to rL LLVM.
Zoxc updated this revision to Diff 25444.May 10 2015, 10:21 PM
Zoxc removed rL LLVM as the repository for this revision.
majnemer added inline comments.May 10 2015, 10:56 PM
lib/Target/X86/X86FrameLowering.cpp
502–506

Please combine this logic with the if/else-if chain.

532–534

Can you use getSUBrrOpcode here?

888

Please capitalize variable names.

903

(Is64Bit ? 8 : 4) is just SlotSize. Also, this looks formatted strangely; consider running clang-format on this.

test/CodeGen/X86/stack-probes.ll
2–3

FIleCheck prefixes are usually all caps.

majnemer added a subscriber: Unknown Object (MLST).May 10 2015, 10:57 PM
Zoxc updated this revision to Diff 25447.May 11 2015, 12:02 AM

Addressed comments

majnemer added inline comments.May 11 2015, 6:54 PM
include/llvm/CodeGen/MachineFunction.h
290

I think this part should be '\brief'

291–294

This bit should just be with the definition as per the coding standard: http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments

lib/Target/X86/X86FrameLowering.cpp
512–513

Please place braces around this else

919–920

This comment should probably be with the NumBytesAdj bit above. Please update the -4 bit as well.

Zoxc updated this revision to Diff 25545.May 11 2015, 7:52 PM

Addressed comments again

Zoxc updated this revision to Diff 29033.Jul 3 2015, 11:40 AM

Rebased

Zoxc updated this revision to Diff 31608.Aug 8 2015, 8:07 PM

The guard region size is passed in RBX/EBX to __probestack. R11 and RBX/EBX is properly spilled if live.

majnemer edited edge metadata.Aug 12 2015, 3:49 PM

Can we have a test which exercises the [RE]BX case?

lib/Target/X86/X86FrameLowering.cpp
207

getX86SubSuperRegister(CheckReg, MVT::i32) can be hoisted out of the loop.

438–448

Please make this an early return, it would reduce indentation.

441

Comments should be complete sentences and end with punctuation.

446

Please end this sentence with a period.

447

Please use SlotSize here.

458–470

Please make this an early return, it would reduce indentation.

883–885

I think this deserves a comment.

890

I think this also deserves a comment.

Zoxc updated this revision to Diff 32035.Aug 13 2015, 1:21 AM
Zoxc edited edge metadata.

Addressed comments.

tamird added a subscriber: tamird.Oct 29 2015, 10:48 AM