This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add address space for SS segment
ClosedPublic

Authored by mlemay-intel on Feb 10 2016, 1:24 PM.

Details

Summary

This patch adds support for address space #258 to refer to the stack
segment (SS). It is analogous to address spaces #256 and #257, which
refer to the GS and FS segments, respectively. This can be used to
direct data accesses to the stack segment, which may be necessary when
a multi-segment memory model is in use.

For example, if the stack segment has a different base address than
the data segment, then it is necessary to output a segment override
prefix to direct a stack access to the stack segment if the memory
operand uses a base address register other than ESP or EBP.
Otherwise, the memory access would be directed to the data segment by
default, which would result in the wrong linear memory address being
accessed.

An example of a system configuration that would result in using
different base addresses for stack and data segments is one in which
the stack and data segments are separated to prevent accesses to the
data segment from corrupting a safe stack that is setup and maintained
by the SafeStack instrumentation pass in Clang. See the SafeStack
runtime library source comments for more details.

Note that although this patch provides the basic support for the stack
segment address space, it does not perform any address space casts
into this address space.

Diff Detail

Repository
rL LLVM

Event Timeline

mlemay-intel retitled this revision from to [X86] Add address space for SS segment.
mlemay-intel updated this object.
delena added a subscriber: delena.Feb 22 2016, 11:33 PM
delena added inline comments.
test/CodeGen/X86/movss.ll
1 ↗(On Diff #47512)

Please use FileCheck instead of grep

Use FileCheck instead of grep in test.

mkuper added inline comments.Apr 21 2016, 6:09 PM
lib/Target/X86/X86ISelDAGToDAG.cpp
1427 ↗(On Diff #53923)

There's also similar code in X86DAGToDAGISel::matchLoadInAddress().
Did you leave that unchanged on purpose?

If yes, perhaps it's worth adding a comment there for why. If not, perhaps factor it out into a separate function instead of having it duplicated 3 times?

test/CodeGen/X86/movss.ll
5 ↗(On Diff #53923)

Can you make the test slightly more explicit?
The current check would match a "movss" instruction, for example.

Also, it would be nice to make it slightly smaller, e.g. something like

define i32 @foo(i32 addrspace(258)* %p) {
  %tmp = load i32, i32 addrspace(258)* %p
  ret i32 %tmp
}

Added comment to make it clear that a related piece of code was not overlooked.
Streamlined test and made it more precise.

Update documentation.

mkuper accepted this revision.Apr 23 2016, 11:40 AM
mkuper edited edge metadata.

LGTM

This revision is now accepted and ready to land.Apr 23 2016, 11:40 AM

LGTM

@mkuper: Can you please land this and D19458? Thanks!

This revision was automatically updated to reflect the committed changes.