Reorder J9ZOS390 conditionals in AtomicSupport.hpp for Open XL z/OS #7640
Conversation
Deigue
force-pushed
the
openxl-fixaix
branch
2 times, most recently
from
94f4f8a to
e845a75
Compare
|
@joransiu I attempted using the atomic Builtins , and read up on the memory options after asking around for some help. Using different variants of memory options and trying a call similar to this restricted to open-xl But seems to be causing a hang at the end of the build, at the same place where explode-image would complete. |
|
Updated PR to remove workaround that was in place. |
|
@0xdaryl Do the revised changes look good? I removed the workaround that was there for the older version of zOS, have kept the atomic suggestions away as it seemed to break the build. |
When Open XL AIX support was merged earlier, it seems to have caused problems with Open XL z/OS builds. The __open_xl__ conditionals meant for AIX are taking priority over the J9ZOS390 clauses, causing problems for z/OS. Was recommended to reorder these so that the more granular J9ZOS390 conditional is encountered and entered first. The old volatile uint32_t workaround seems to be no longer valid as the minimal zos version is 2.3, and is removed after testing it works without the workaround. Signed-off-by: Gaurav Chaudhari <[email protected]>
|
@babsingh Addressed the feedback from above, and corrected some additional comments around conditionals. |
|
jenkins build all |
| /* Call __csg directly as csg() does not exist. */ | ||
| __csg((void*)&oldValue, (void*)address, (void*)&newValue); | ||
| return oldValue; | ||
| #else /* defined(OMR_ENV_DATA64) */ | ||
| /* __cds1 does not write the swap value correctly, cds does the correct thing. */ | ||
| cds((cds_t*)&oldValue, (cds_t*)address, *(cds_t*)&newValue); | ||
| return oldValue; |
There was a problem hiding this comment.
The indentation is off. It doesn't match line 418 or 431.
| /* 390 cs() function defined in <stdlib.h>, doesn't expand properly to __cs1() which correctly deals with aliasing. */ | ||
| __cs1((uint32_t *)&oldValue, (uint32_t *)address, (uint32_t *)&newValue); | ||
| return oldValue; |
There was a problem hiding this comment.
The indentation is off over here as well. It doesn't match line 354 or 360.
When Open XL AIX support was merged earlier, it seems to have caused problems with Open XL z/OS builds. The open_xl conditionals meant for AIX are taking priority over the J9ZOS390 clauses, causing problems for z/OS. Was recommended to reorder these so that the more granular J9ZOS390 conditional is encountered and entered first.