Merge pull request #298 from tactcomplabs/fix_stack_thread_allocation

Fix stack and thread local storage allocation

Author: leekillough

include/RevCPU.h

@@ -235,9 +235,6 @@ class RevCPU : public SST::Component {
   // - Handles updating LSQueue & MarkLoadComplete function pointers
   void AssignThread( std::unique_ptr<RevThread>&& ThreadToAssign, unsigned ProcID );
 
-  // Sets up arguments for a thread with a given ID and feature set.
-  void SetupArgs( const std::unique_ptr<RevRegFile>& RegFile );
-
   // Checks the status of ALL threads that are currently blocked.
   void CheckBlockedThreads();
 

include/RevRegFile.h

@@ -45,6 +45,7 @@ inline void BoxNaN( double* dest, const float* value ) {
 enum class RevReg : uint16_t {
   zero =  0, ra  =  1, sp   =  2, gp   =  3, tp  =  4, t0  =  5, t1   =  6, t2   =  7,
   s0   =  8, s1  =  9, a0   = 10, a1   = 11, a2  = 12, a3  = 13, a4   = 14, a5   = 15,
+  fp   =  8,
   a6   = 16, a7  = 17, s2   = 18, s3   = 19, s4  = 20, s5  = 21, s6   = 22, s7   = 23,
   s8   = 24, s9  = 25, s10  = 26, s11  = 27, t3  = 28, t4  = 29, t5   = 30, t6   = 31,
   ft0  =  0, ft1 =  1, ft2  =  2, ft3  =  3, ft4 =  4, ft5 =  5, ft6  =  6, ft7  =  7,

src/RevCPU.cc

@@ -694,10 +694,12 @@ bool RevCPU::clockTick( SST::Cycle_t currentCycle ) {
 void RevCPU::InitThread( std::unique_ptr<RevThread>&& ThreadToInit ) {
   // Get a pointer to the register state for this thread
   std::unique_ptr<RevRegFile> RegState = ThreadToInit->TransferVirtRegState();
+
   // Set the global pointer register and the frame pointer register
   auto gp                              = Loader->GetSymbolAddr( "__global_pointer$" );
   RegState->SetX( RevReg::gp, gp );
-  RegState->SetX( RevReg::s0, gp );  // s0 = x8 = frame pointer register
+  RegState->SetX( RevReg::fp, gp );
+
   // Set state to Ready
   ThreadToInit->SetState( ThreadState::READY );
   output.verbose( CALL_INFO, 4, 0, "Initializing Thread %" PRIu32 "\n", ThreadToInit->GetID() );
@@ -754,18 +756,6 @@ void RevCPU::CheckBlockedThreads() {
   return;
 }
 
-// ----------------------------------
-// We need to initialize the x10 register to include the value of
-// ARGC. This is >= 1 (the executable name is always included).
-// We also need to initialize the ARGV pointer to the value
-// of the ARGV base pointer in memory which is currently set to
-// the top of stack.
-// ----------------------------------
-void RevCPU::SetupArgs( const std::unique_ptr<RevRegFile>& RegFile ) {
-  RegFile->SetX( RevReg::a0, Opts->GetArgv().size() + 1 );
-  RegFile->SetX( RevReg::a1, Mem->GetStackTop() );
-}
-
 // Checks core 'i' to see if it has any available harts to assign work to
 // if it does and there is work to assign (ie. ReadyThreads is not empty)
 // assign it and enable the processor if not already enabled.
@@ -860,14 +850,35 @@ void RevCPU::HandleThreadStateChangesForProc( uint32_t ProcID ) {
 }
 
 void RevCPU::InitMainThread( uint32_t MainThreadID, const uint64_t StartAddr ) {
-  // @Lee: Is there a better way to get the feature info?
-  std::unique_ptr<RevRegFile> MainThreadRegState = std::make_unique<RevRegFile>( Procs[0] );
+  auto MainThreadRegState = std::make_unique<RevRegFile>( Procs[0] );
+
+  // The Program Counter gets set to the start address
   MainThreadRegState->SetPC( StartAddr );
-  MainThreadRegState->SetX( RevReg::tp, Mem->GetThreadMemSegs().front()->getTopAddr() );
-  MainThreadRegState->SetX( RevReg::sp, Mem->GetThreadMemSegs().front()->getTopAddr() - Mem->GetTLSSize() );
-  MainThreadRegState->SetX( RevReg::gp, Loader->GetSymbolAddr( "__global_pointer$" ) );
-  MainThreadRegState->SetX( 8, Loader->GetSymbolAddr( "__global_pointer$" ) );
-  SetupArgs( MainThreadRegState );
+
+  // We need to initialize the a0 register to the value of ARGC.
+  // This is >= 1 (the executable name is always included).
+  // We also need to initialize the a1 register to the ARGV pointer
+  // of the ARGV base pointer in memory which is currently set to
+  // the top of stack.
+  uint64_t stackTop = Mem->GetStackTop();
+  MainThreadRegState->SetX( RevReg::a0, Opts->GetArgv().size() + 1 );
+  MainThreadRegState->SetX( RevReg::a1, stackTop );
+
+  // We subtract Mem->GetTLSSize() bytes from the current stack top, and
+  // round down to a multiple of 16 bytes. We set it as the new top of stack.
+  stackTop = ( stackTop - Mem->GetTLSSize() ) & ~uint64_t{ 15 };
+  Mem->SetStackTop( stackTop );
+
+  // We set the stack pointer and the thread pointer to the new stack top
+  // The thread local storage is accessed with a nonnegative offset from tp,
+  // and the stack grows down with sp being subtracted from before storing.
+  MainThreadRegState->SetX( RevReg::sp, stackTop );
+  MainThreadRegState->SetX( RevReg::tp, stackTop );
+
+  auto gp = Loader->GetSymbolAddr( "__global_pointer$" );
+  MainThreadRegState->SetX( RevReg::gp, gp );
+  MainThreadRegState->SetX( RevReg::fp, gp );
+
   std::unique_ptr<RevThread> MainThread = std::make_unique<RevThread>(
     MainThreadID,
     _INVALID_TID_,  // No Parent Thread ID

src/RevCore.cc

@@ -1916,25 +1916,30 @@ RevRegFile* RevCore::GetRegFile( unsigned HartID ) const {
 void RevCore::CreateThread( uint32_t NewTID, uint64_t firstPC, void* arg ) {
   // tidAddr is the address we have to write the new thread's id to
   output->verbose( CALL_INFO, 2, 0, "Creating new thread with PC = 0x%" PRIx64 "\n", firstPC );
-  uint32_t ParentThreadID                      = Harts.at( HartToExecID )->GetAssignedThreadID();
+  uint32_t ParentThreadID                  = Harts.at( HartToExecID )->GetAssignedThreadID();
 
   // Create the new thread's memory
-  std::shared_ptr<MemSegment> NewThreadMem     = mem->AddThreadMem();
+  std::shared_ptr<MemSegment> NewThreadMem = mem->AddThreadMem();
 
   // TODO: Copy TLS into new memory
 
   // Create new register file
-  std::unique_ptr<RevRegFile> NewThreadRegFile = std::make_unique<RevRegFile>( this );
+  auto NewThreadRegFile                    = std::make_unique<RevRegFile>( this );
 
   // Copy the arg to the new threads a0 register
   NewThreadRegFile->SetX( RevReg::a0, reinterpret_cast<uintptr_t>( arg ) );
 
+  // Set the stack and thread pointer
+  // The thread local storage is accessed with a nonnegative offset from tp,
+  // and the stack grows down with sp being subtracted from before storing.
+  uint64_t sp = ( NewThreadMem->getTopAddr() - mem->GetTLSSize() ) & ~uint64_t{ 15 };
+  NewThreadRegFile->SetX( RevReg::tp, sp );
+  NewThreadRegFile->SetX( RevReg::sp, sp );
+
   // Set the global pointer
-  // TODO: Cleanup
-  NewThreadRegFile->SetX( RevReg::tp, NewThreadMem->getTopAddr() );
-  NewThreadRegFile->SetX( RevReg::sp, NewThreadMem->getTopAddr() - mem->GetTLSSize() );
-  NewThreadRegFile->SetX( RevReg::gp, loader->GetSymbolAddr( "__global_pointer$" ) );
-  NewThreadRegFile->SetX( RevReg::s0, loader->GetSymbolAddr( "__global_pointer$" ) );
+  auto gp = loader->GetSymbolAddr( "__global_pointer$" );
+  NewThreadRegFile->SetX( RevReg::gp, gp );
+  NewThreadRegFile->SetX( RevReg::fp, gp );  // frame pointer register
   NewThreadRegFile->SetPC( firstPC );
 
   // Create a new RevThread Object

src/RevLoader.cc

@@ -478,8 +478,8 @@ bool RevLoader::LoadProgramArgs( const std::string& exe, const std::vector<std::
   //
   // Each string that argv[i] points to starts at an address which is a multiple of XLEN.
   //
-  // Finally, in RevCPU::SetupArgs(), we initialize the a0 register to the value of argc
-  // and the a1 register to the base pointer to argv.
+  // Finally, in RevCPU::InitMainThread(), we initialize the a0 register to the value of
+  // argc and the a1 register to the base pointer to argv.
   // -------------- END MEMORY LAYOUT NOTES
 
   // Allocate sizeof(XLEN) bytes for each of the argv[] pointers

test/CMakeLists.txt

@@ -230,6 +230,7 @@ add_rev_test(ARGC_SHORT argc_short 30 "test_level=2;memh;rv64;loader;" SCRIPT "r
 add_rev_test(ARGV argv 30 "test_level=2;memh;rv64;loader;c++" SCRIPT "run_argv.sh")
 add_rev_test(ARGV_layout argv_layout 30 "test_level=2;memh;rv32;loader;" SCRIPT "run_argv_layout.sh")
 add_rev_test(ARGV_limit argv_limit 75 "test_level=2;memh;rv32;loader;" SCRIPT "run_argv_limit.sh")
+add_rev_test(ARGV_stack argv_stack 10 "memh;rv64")
 add_rev_test(COPROC_EX coproc_ex 30 "memh;rv64;coproc" SCRIPT "run_coproc_ex.sh")
 
 if(TEST_LEVEL GREATER_EQUAL 2)

test/argv_stack/Makefile

@@ -0,0 +1,25 @@
+#
+# Makefile
+#
+# makefile: argv_stack
+#
+# Copyright (C) 2017-2024 Tactical Computing Laboratories, LLC
+# All Rights Reserved
+# [email protected]
+#
+# See LICENSE in the top level directory for licensing details
+#
+
+.PHONY: src
+
+EXAMPLE=argv_stack
+CC=${RVCC}
+ARCH=rv64imafdc
+
+all: $(EXAMPLE).exe
+$(EXAMPLE).exe: $(EXAMPLE).c
+	$(CC) -march=$(ARCH) -O3 -static -o $(EXAMPLE).exe $(EXAMPLE).c
+clean:
+	rm -Rf $(EXAMPLE).exe
+
+#-- EOF

test/argv_stack/argv_stack.c

@@ -0,0 +1,17 @@
+/*
+ * argv_stack.c
+ *
+ * Copyright (C) 2017-2024 Tactical Computing Laboratories, LLC
+ * All Rights Reserved
+ * [email protected]
+ *
+ * See LICENSE in the top level directory for licensing details
+ *
+ */
+
+int main( int argc, char** argv ) {
+  char a[1024];
+  if( a + sizeof( a ) > (char*) argv )
+    asm( ".word 0" );
+  return 0;
+}