[MDEV-29324] Compile error in debug mode on s390x (srw_lock.cc) Created: 2022-08-18  Updated: 2023-09-11  Resolved: 2023-09-07

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.8.3
Fix Version/s: 10.6.16, 10.10.7, 10.11.6, 11.0.4, 11.1.3, 11.2.2

Type: Bug Priority: Major
Reporter: Vivian Kong Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: None
Environment:

s390x


Issue Links:
Relates
relates to MDEV-27956 hardware lock elision on s390x / ppc6... Closed

 Description   

Tried to compile the server in debug mode on s390x but I ran into a compile error in srw_lock.cc (https://github.com/MariaDB/server/blob/10.11/storage/innobase/sync/srw_lock.cc#L121).

[ 65%] Building CXX object storage/innobase/CMakeFiles/innobase.dir/sync/srw_lock.cc.o
/home/tester/server/storage/innobase/sync/srw_lock.cc: In function ?bool xtest()?:
/home/tester/server/storage/innobase/sync/srw_lock.cc:121:17: error: ?__builtin_ttest? was not declared in this scope; did you mean ?__builtin_tend??
  121 |     _HTM_STATE (__builtin_ttest ()) == _HTM_TRANSACTIONAL;
      |                 ^~~~~~~~~~~~~~~
      |                 __builtin_tend
/home/tester/server/storage/innobase/sync/srw_lock.cc:121:5: error: ?_HTM_STATE? was not declared in this scope
  121 |     _HTM_STATE (__builtin_ttest ()) == _HTM_TRANSACTIONAL;
      |     ^~~~~~~~~~
/home/tester/server/storage/innobase/sync/srw_lock.cc:121:40: error: ?_HTM_TRANSACTIONAL? was not declared in this scope
  121 |     _HTM_STATE (__builtin_ttest ()) == _HTM_TRANSACTIONAL;
      |                                        ^~~~~~~~~~~~~~~~~~
make[2]: *** [storage/innobase/CMakeFiles/innobase.dir/build.make:1406: storage/innobase/CMakeFiles/innobase.dir/sync/srw_lock.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:5108: storage/innobase/CMakeFiles/innobase.dir/all] Error 2
make: *** [Makefile:166: all] Error 2



 Comments   
Comment by Daniel Black [ 2022-08-21 ]

__builtin_tx_nesting_depth() > 0 as a s390x equivalent version of the expression maybe ?

https://gcc.gnu.org/onlinedocs/gcc/S_002f390-System-z-Built-in-Functions.html#S_002f390-System-z-Built-in-Functions

Comment by Ehsan Kiani Far [ 2023-06-30 ]

I replaced this part of code with __builtin_tx_nesting_depth() > 0 as Daniel suggested. The debug build pass. I am looking for a way to check if that part of code serves its intended purpose.

Comment by Ehsan Kiani Far [ 2023-08-01 ]

I wrote a PoC code to check Daniel's proposed solution:

#include <iostream>
#include <s390intrin.h>
# include <htmxlintrin.h>
 
int do_transactional_operation(int* data, int value, bool go_deeper) {
    int depth = 0;
    //start transaction!
    if (__TM_simple_begin() ==  _HTM_TBEGIN_STARTED) {
        //do operation in TM!
        *data = value;
        //read the nestig depth
        depth = __builtin_tx_nesting_depth();
        if (go_deeper)
        {
            depth = do_transactional_operation(data, 888, false);
        }
        //commit!
        __TM_end(); 
    }
    return depth;
}
 
int do_non_transactional_operation()
{
        return __builtin_tx_nesting_depth();
}
 
int main() {
    int shared_data = 0;
 
    int transaction_depth_0 = do_non_transactional_operation();
    std::cout << "Transaction 0: " << transaction_depth_0  << "   Shared data: " << shared_data << std::endl;
 
    int transaction_depth_1 = do_transactional_operation(&shared_data, 777, false);
    std::cout << "Transaction 1: " << transaction_depth_1  << "   Shared data: " << shared_data << std::endl;
 
    int transaction_depth_2 = do_transactional_operation(&shared_data, 777, true);
    std::cout << "Transaction 2: " << transaction_depth_2  << "   Shared data: " << shared_data << std::endl;
 
    return 0;
}

After compiling and running on s390x, I got this result:

Transaction 0: 0   Shared data: 0
Transaction 1: 1   Shared data: 777
Transaction 2: 2   Shared data: 888

It looks correct to me and must be safe to PR that change and merge. What do you think @Daniel?

Comment by Daniel Black [ 2023-08-13 ]

Sure, feel free to target 10.5 with the appropriate pre-processor directives for s390x

Comment by Nayana [ 2023-08-30 ]

srw_lock.cc file is added in v10.6 .
https://github.com/MariaDB/server/commit/1fdc161d8faeb18acf0ccea9b33ad64f0b596f79#diff-13da78db7fb5204cfaa088afbd124d86be0b8f05eb1ded19b0a00fcd20576206

Comment by Daniel Black [ 2023-08-30 ]

quite right. Looking forward to PR.

Comment by Nayana [ 2023-09-04 ]

PR raised - https://github.com/MariaDB/server/pull/2745

Comment by Marko Mäkelä [ 2023-09-04 ]

Thank you. Based on what I found, the __builtin_tx_nesting_depth() is specific to the s390x (not to POWER) and it was introduced at the same time with the htm target attribute, in GCC 6.5 (or possibly in some earlier release of GCC 6; the documentation I found was for GCC 6.5).

By the way, there is a unit test innodb_sync-t that should exercise this.

cmake -DWITH_UNIT_TESTS=ON .
cmake --build .
storage/innobase/unittest/innodb_sync-t
cd mysql-test
./mtr --parallel=auto --suite=unit

The unit test program innodb_sync-t is run by the test unit.innodb_sync.

Comment by Nayana [ 2023-09-11 ]

Executed above steps and all tests are passing.

Generated at Thu Feb 08 10:07:38 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.