This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][libomptarget] Add missing symbols in dynamic_hsa
ClosedPublic

Authored by kevinsala on Dec 16 2022, 6:32 AM.

Details

Summary

This patch prepares for the new AMDGPU NextGen plugin.

Diff Detail

Event Timeline

kevinsala created this revision.Dec 16 2022, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 6:32 AM
kevinsala requested review of this revision.Dec 16 2022, 6:32 AM
jhuber6 accepted this revision.Dec 16 2022, 6:33 AM
This revision is now accepted and ready to land.Dec 16 2022, 6:33 AM

If something has a simple uncontroversial fix, it's fine to just push it directly without review. Usually easier than reverting a whole patch.

arsenm added a subscriber: arsenm.Dec 16 2022, 6:39 AM
arsenm added inline comments.
openmp/libomptarget/plugins/amdgpu/dynamic_hsa/hsa.h
306

Is there a reason this is missing one?

kevinsala marked an inline comment as done.Dec 16 2022, 6:41 AM
kevinsala added inline comments.
openmp/libomptarget/plugins/amdgpu/dynamic_hsa/hsa.h
306
jhuber6 added inline comments.Dec 16 2022, 6:41 AM
openmp/libomptarget/plugins/amdgpu/dynamic_hsa/hsa.h
306
typedef enum {                                                                                                                                                                                                    
  /**                                                                                                                                                                                                             
   * Use a default floating-point rounding mode specified elsewhere.                                                                                                                                              
   */                                                                                                                                                                                                             
  HSA_DEFAULT_FLOAT_ROUNDING_MODE_DEFAULT = 0,    
  /**                                                                                                                                                                                                             
   * Operations that specify the default floating-point mode are rounded to zero                                                                                                                                  
   * by default.                                                                                                                                                                                                  
   */                                                                                                                                                                                                             
  HSA_DEFAULT_FLOAT_ROUNDING_MODE_ZERO = 1,                                                                                                                                                                       
  /**                                                                                                                                                                                                             
   * Operations that specify the default floating-point mode are rounded to the                                                                                                                                   
   * nearest representable number and that ties should be broken by selecting                                                                                                                                     
   * the value with an even least significant bit.                                                                                                                                                                
   */                                                                                                                                                                                                             
  HSA_DEFAULT_FLOAT_ROUNDING_MODE_NEAR = 2                                                                                                                                                                        
} hsa_default_float_rounding_mode_t;

That's all I see in my installation, unless this was changed recently.

arsenm added inline comments.Dec 16 2022, 6:43 AM
openmp/libomptarget/plugins/amdgpu/dynamic_hsa/hsa.h
306

There are 4 IEEE rounding modes and the hardware supports all of them so this is suspicious. Also "default" = near, so really this is missing +/- infinity

jhuber6 added inline comments.
openmp/libomptarget/plugins/amdgpu/dynamic_hsa/hsa.h
306

That's a good point, Clang defined the following, which matches what I just looked up at https://en.wikipedia.org/wiki/IEEE_754#Rounding_rules.

TowardZero        = 0,    ///< roundTowardZero.                                                                                                                                     
NearestTiesToEven = 1,    ///< roundTiesToEven.                                                                                                                                           
TowardPositive    = 2,    ///< roundTowardPositive.                                                                                                                      
TowardNegative    = 3,    ///< roundTowardNegative.                                                                                                          
NearestTiesToAway = 4,    ///< roundTiesToAway.

These three are all that's included in hsa.h for when we load the executable, not sure who would be the person to bring that up to, maybe @JonChesterfield has an idea. Maybe they're combining the +/- and nearest rounding modes somehow.

In any case this is unrelated to this patch as it just copied what hsa.h gives it.

This header file is a compromise between including enough of hsa.h to compile without the rocm source and not including all of it because it's large. Different people have taken different approaches to removing unused parts. There's a similar cuda.h near nvptx.

This revision was automatically updated to reflect the committed changes.
kevinsala marked an inline comment as done.