WIP: Apple Silicon -- possible changes to TdiExtFunction.c

[mwinkel-dev]

The MDSplus website claims that TDI's EXT_FUNCTION() has three modes of operation. However, it appears that both the documentation and code are broken. Thus it is questionable if this routine should be ported to Apple Silicon.

The three modes:

1. EXT_FUNCTION(,routine, ...) = this works and is used extensively by MDSplus to execute *.fun files.
2. EXT_FUNCTION(image, routine, ...) = duplicates BUILD_CALL (aka ->), segfaults on Ubuntu22 x86-64
3. EXT_FUNCTION(tdi_fun_file, ...) = doesn't seem to work, but didn't test it extensively.
   EXT_FUNCTION page

Here are the options (listed in order of recommendation):
a) Leave "as is" = executes *.fun files A-OK
b) delete all code associate with 2).
c) fix all three modes and port to Apple Silicon = however 2) has probably been broken for years, so why fix it and duplicate what BUILD_CALL already does?

If we choose option c), then the libffi code would be something along the lines of the WIP draft.

[joshStillerman]

The only usages of this that I have ever seen are:

• .fun files
• .py files
• image->routine

My feeling is that these are the only cases that we need to maintain.

[mwinkel-dev]

Josh recommended deleting the obsolete / broken code for the EXT_FUNCTION(<library>,<function>) mode.

As an experiment, that portion of TdiExtFunction.c was commented out. Thereafter, it still passed 100% of the automated tests.

[WhoBrokeTheBuild]

I'm hesitantly in favor of this as well, we can always add it back if we need.

[mwinkel-dev]

For the record, here is the experiment conducted with the existing alpha branch (no Apple Silicon code) on Ubuntu24 (x86-64). This is similar to the TdiShr->TdiPi test in the Mds.java file, which does work OK using BUILD_CALL (aka ->).

[mwinkel@mfews-mwinkel repos]$ tdic
TDI> ext_function('TdiShr', 'TdiPi', val(0), val(1))
zsh: segmentation fault (core dumped)  tdic

[mwinkel-dev]

Turns out portions of the EXT_FUNCTION(<library>,<function>) are likely needed. Deleted that code and had it return a MDSplusERROR code whenever it received a library name. Whereupon the MdsExpressionCompileTest.cpp automated test failed on an ext_function('TreeShr','TreeCtx') test.

Running that test on Ubuntu24 (x86-64) with the alpha branch appears to work.

$ tdic                   
TDI> ext_function('TreeShr', 'TreeCtx')
Pointer(0x63cb2faf6360)

So now investigating why the automated tests pass 100% when leave the code "as is" or comment it out. True, the addition of the MDSplusERROR is the obvious cause of the failing test. However it reveals that there are likely multiple execution flows in the EXT_FUNCTION(<library>,<function>) code, most of which do not involve LibCallg() thus work fine on Apple Silicon.

[mwinkel-dev]

Experiments show that the EXT_FUNCTION(<library>, <function>, ...) feature only works if <function> returns a pointer. If it instead return an int or other data value, the MdsCopyDxXd() after the LibCallg() segfaults.

Even if a pointer is returned (such as int *) to EXT_FUNCTION(), it doesn't appear to be easy (or possible?) for TDI to convert a DTYPE_POINTER to a useful data type (such as DTYPE_L). If the EXT_FUNCTION(<library>, <function>, ...) feature was widely used, there would already have been many bug reports filed on this limitation. However, as that is not the case, the conclusion is that this feature is seldom used.

Therefore as per discussion with @WhoBrokeTheBuild, the EXT_FUNCTION(<library>, <function>, ...) feature has been removed. It is now replaced with a brief warning message explaining that BUILD_CALL (aka ->) should be used instead and then throws an MDSplusERROR.

The ext_function('TreeShr', 'TreeCtx') in the automated test suite has been rewritten as TreeShr->TreeCtx().

See PR #2877 for the details of the above changes.

[mwinkel-dev]

This change was included in PR #2877, so closing this.